From b1c09c0c1e3caf29821d70e81defe5c66ac097bf Mon Sep 17 00:00:00 2001 From: Edwin Buck Date: Fri, 2 Feb 2024 09:45:41 -0600 Subject: [PATCH 01/20] Initial support for dynamically adjustable log levels. Add list command to cli. Add some of the receiving services in the server "run" instance. Add the "registry" of loggers (currently a btree) Beginning unit testing on naming ordering. Signed-off-by: Edwin Buck --- cmd/spire-server/cli/cli.go | 4 + cmd/spire-server/cli/logger/listCommand.go | 127 +++++++++++++++++++++ cmd/spire-server/util/util.go | 6 + pkg/common/api/middleware/names.go | 1 + pkg/common/logger/logger.go | 49 ++++++++ pkg/common/logger/logger_test.go | 42 +++++++ pkg/server/endpoints/endpoints.go | 7 +- 7 files changed, 234 insertions(+), 2 deletions(-) create mode 100644 cmd/spire-server/cli/logger/listCommand.go create mode 100644 pkg/common/logger/logger.go create mode 100644 pkg/common/logger/logger_test.go diff --git a/cmd/spire-server/cli/cli.go b/cmd/spire-server/cli/cli.go index 93b5447cda..c85372d241 100644 --- a/cmd/spire-server/cli/cli.go +++ b/cmd/spire-server/cli/cli.go @@ -11,6 +11,7 @@ import ( "github.com/spiffe/spire/cmd/spire-server/cli/federation" "github.com/spiffe/spire/cmd/spire-server/cli/healthcheck" "github.com/spiffe/spire/cmd/spire-server/cli/jwt" + "github.com/spiffe/spire/cmd/spire-server/cli/logger" "github.com/spiffe/spire/cmd/spire-server/cli/run" "github.com/spiffe/spire/cmd/spire-server/cli/token" "github.com/spiffe/spire/cmd/spire-server/cli/validate" @@ -96,6 +97,9 @@ func (cc *CLI) Run(ctx context.Context, args []string) int { "federation update": func() (cli.Command, error) { return federation.NewUpdateCommand(), nil }, + "logger list": func() (cli.Command, error) { + return logger.NewListCommand(), nil + }, "run": func() (cli.Command, error) { return run.NewRunCommand(ctx, cc.LogOptions, cc.AllowUnknownConfig), nil }, diff --git a/cmd/spire-server/cli/logger/listCommand.go b/cmd/spire-server/cli/logger/listCommand.go new file mode 100644 index 0000000000..732cd8ed79 --- /dev/null +++ b/cmd/spire-server/cli/logger/listCommand.go @@ -0,0 +1,127 @@ +package logger + +import ( + "context" + "errors" + "flag" + "fmt" + + "github.com/mitchellh/cli" + api "github.com/spiffe/spire-api-sdk/proto/spire/api/server/logger/v1" + "github.com/spiffe/spire-api-sdk/proto/spire/api/types" + "github.com/spiffe/spire/cmd/spire-server/util" + commoncli "github.com/spiffe/spire/pkg/common/cli" + "github.com/spiffe/spire/pkg/common/cliprinter" +) + +const PAGE_SIZE = 500 + +type listCommand struct { + env *commoncli.Env + printer cliprinter.Printer + serverClient util.ServerClient + + requestRoot string + includeSubloggers bool + pageSize int32 +} + +// Returns a cli.command that lists all of the named loggers +func NewListCommand() cli.Command { + return NewListCommandWithEnv(commoncli.DefaultEnv) +} + +func NewListCommandWithEnv(env *commoncli.Env) cli.Command { + return util.AdaptCommand(env, &listCommand{env: env}) +} + +func (_ *listCommand) Name() string { + return "logger list" +} + +func (_ *listCommand) Synopsis() string { + return "Lists loggers and their log levels" +} + +func (c *listCommand) Run(ctx context.Context, _ *commoncli.Env, serverClient util.ServerClient) error { + c.serverClient = serverClient + + var err error // predeclare for post loop error processing + response := &api.ListLoggersResponse{} + for page, err := c.pageRequest(ctx, ""); + err != nil && page.NextPageToken != ""; + page, err = c.pageRequest(ctx, page.NextPageToken) { + response.Loggers = append(response.Loggers, page.Loggers...) + } + if err != nil { + return fmt.Errorf("error fetching loggers: %w", err) + } + + return nil +} + +// Requests just one page. +func (c *listCommand) pageRequest(ctx context.Context, pageToken string) (*api.ListLoggersResponse, error) { + return c.serverClient.NewLoggerClient().ListLoggers(ctx, &api.ListLoggersRequest{ + RootName: c.requestRoot, + WithSubloggers: c.includeSubloggers, + PageSize: c.pageSize, + PageToken: pageToken, + }) +} + +func (c *listCommand) AppendFlags(fs *flag.FlagSet) { + fs.StringVar(&c.requestRoot, "name", "", + "The name of the logger (\"\" for root)") + fs.BoolVar(&c.includeSubloggers, "subloggers", true, + "Include subloggers of \"name\"") + cliprinter.AppendFlagWithCustomPretty(&c.printer, fs, c.env, c.prettyPrintLoggers) +} + +func (l* listCommand) prettyPrintLoggers(env *commoncli.Env, results ...any) error { + response, ok := results[0].(*api.ListLoggersResponse) + if !ok { + return errors.New("internal error: print list cli printer; please report this bug") + } + + count := len(response.Loggers) + switch count { + case 0: + if err := env.Printf("No loggers found\n"); err != nil { + return err + } + case 1: + if err := env.Printf("Found 1 logger\n"); err != nil { + return err + } + default: + if err := env.Printf("Found %d loggers\n", count); err != nil { + return err + } + if err := env.Println(); err != nil { + return err + } + for _, logger := range response.Loggers { + if err := l.printLogger(env, logger); err != nil { + return err + } + } + } + return nil +} + +func (l *listCommand) printLogger(env *commoncli.Env, logger *types.Logger) error { + if err := env.Printf("Logger Name : %s\n", logger.Name); err != nil { + return err + } + if err := env.Printf("Logger Level : %d\n", logger.CurrentLevel); err != nil { + return err + } + if err := env.Printf("Logger Default : %d\n", logger.DefaultLevel); err != nil { + return err + } + if err := env.Println(); err != nil { + return err + } + return nil +} diff --git a/cmd/spire-server/util/util.go b/cmd/spire-server/util/util.go index 791359f49d..258dfbc1c3 100644 --- a/cmd/spire-server/util/util.go +++ b/cmd/spire-server/util/util.go @@ -14,6 +14,7 @@ import ( agentv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/agent/v1" bundlev1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/bundle/v1" entryv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/entry/v1" + loggerv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/logger/v1" svidv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/svid/v1" trustdomainv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/trustdomain/v1" api_types "github.com/spiffe/spire-api-sdk/proto/spire/api/types" @@ -45,6 +46,7 @@ type ServerClient interface { NewAgentClient() agentv1.AgentClient NewBundleClient() bundlev1.BundleClient NewEntryClient() entryv1.EntryClient + NewLoggerClient() loggerv1.LoggerClient NewSVIDClient() svidv1.SVIDClient NewTrustDomainClient() trustdomainv1.TrustDomainClient NewHealthClient() grpc_health_v1.HealthClient @@ -78,6 +80,10 @@ func (c *serverClient) NewEntryClient() entryv1.EntryClient { return entryv1.NewEntryClient(c.conn) } +func (c *serverClient) NewLoggerClient() loggerv1.LoggerClient { + return loggerv1.NewLoggerClient(c.conn) +} + func (c *serverClient) NewSVIDClient() svidv1.SVIDClient { return svidv1.NewSVIDClient(c.conn) } diff --git a/pkg/common/api/middleware/names.go b/pkg/common/api/middleware/names.go index d09ee0829f..3957403487 100644 --- a/pkg/common/api/middleware/names.go +++ b/pkg/common/api/middleware/names.go @@ -19,6 +19,7 @@ const ( EnvoySDSv3ServiceShortName = "SDS.v3" HealthServiceName = "grpc.health.v1.Health" HealthServiceShortName = "Health" + LoggerServiceName = "logger.v1.Logger" DelegatedIdentityServiceName = "spire.api.agent.delegatedidentity.v1.DelegatedIdentity" DelegatedIdentityServiceShortName = "DelegatedIdentity" ServerReflectionServiceName = "grpc.reflection.v1.ServerReflection" diff --git a/pkg/common/logger/logger.go b/pkg/common/logger/logger.go new file mode 100644 index 0000000000..15177028d9 --- /dev/null +++ b/pkg/common/logger/logger.go @@ -0,0 +1,49 @@ +package logger + +import ( + "strings" + "sync" + + "github.com/google/btree" +) + +type loggerRecord struct { + name string +} + +// True if a is first, false otherwise +func sortByName(lesser, greater loggerRecord) bool { + lesserSegments := strings.Split(lesser.name, ".") + greaterSegments := strings.Split(greater.name, ".") + + var index int // declaration for post loop processing + for index = 0; index < len(lesserSegments); index++ { + // lesser is "... (index) segment", greater is "... (index)" + // fewer segments first, so lesser is not lesser + if index >= len(greaterSegments) { + return false + } + // lesser is "... (index) aaa", greater is "... (index) b" + // lexographic ordering of segments, so lesser is lesser + if lesserSegments[index] < greaterSegments[index] { + return true + } + // lesser is "... (index) b", greater is "... (index) aaa" + // lexographic ordering of segments, so lesser is not lesser + if lesserSegments[index] > greaterSegments[index] { + return false + } + // lesser is "... (index) aaa", greater is "... (index) aaa" + // continue comparison advancing to next segment + } + // lesser is "... (index)", greater is "... (index) aaa" + // fewer segements first, so lesser is lesser + return len(lesserSegments) < len(greaterSegments) +} + +type Registry struct { + mu sync.RWMutex + + loggersByName *btree.BTreeG[loggerRecord] +} + diff --git a/pkg/common/logger/logger_test.go b/pkg/common/logger/logger_test.go new file mode 100644 index 0000000000..9963d5d599 --- /dev/null +++ b/pkg/common/logger/logger_test.go @@ -0,0 +1,42 @@ +package logger + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// Desired order +// "" always first +// a.b before a.b.c +// a.b before a.c +// a.b before b.b +func TestSortByName(t *testing.T) { + var tests = []struct { + First string + Second string + }{ + { "", "a"}, + {"0", "1"}, + {"02", "1"}, + {"1", "a"}, + {"a", "aa"}, + {"a", "ab"}, + {"aa", "ab"}, + {"aa", "abc"}, + {"ab", "abc"}, + {"a.b", "aa.c"}, + {"a.b.c", "aa.a"}, + {"a.b", "a.c"}, + } + for _, testCase := range tests { + first := loggerRecord{ + name: testCase.First, + } + second := loggerRecord{ + name: testCase.Second, + } + require.True(t, sortByName(first, second), "Name %s sould come before %s", first.name, second.name) + require.False(t, sortByName(second, first), "Name %s sould come after %s", second.name, first.name) + } +} diff --git a/pkg/server/endpoints/endpoints.go b/pkg/server/endpoints/endpoints.go index a6c5429758..6bb1e5aa87 100644 --- a/pkg/server/endpoints/endpoints.go +++ b/pkg/server/endpoints/endpoints.go @@ -25,6 +25,7 @@ import ( bundlev1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/bundle/v1" debugv1_pb "github.com/spiffe/spire-api-sdk/proto/spire/api/server/debug/v1" entryv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/entry/v1" + loggerv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/logger/v1" svidv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/svid/v1" trustdomainv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/trustdomain/v1" "github.com/spiffe/spire/pkg/common/auth" @@ -86,6 +87,7 @@ type APIServers struct { DebugServer debugv1_pb.DebugServer EntryServer entryv1.EntryServer HealthServer grpc_health_v1.HealthServer + LoggerServer loggerv1.LoggerServer SVIDServer svidv1.SVIDServer TrustDomainServer trustdomainv1.TrustDomainServer } @@ -175,7 +177,7 @@ func (e *Endpoints) ListenAndServe(ctx context.Context) error { tcpServer := e.createTCPServer(ctx, unaryInterceptor, streamInterceptor) udsServer := e.createUDSServer(unaryInterceptor, streamInterceptor) - // New APIs + // TCP and UDP agentv1.RegisterAgentServer(tcpServer, e.APIServers.AgentServer) agentv1.RegisterAgentServer(udsServer, e.APIServers.AgentServer) bundlev1.RegisterBundleServer(tcpServer, e.APIServers.BundleServer) @@ -187,7 +189,8 @@ func (e *Endpoints) ListenAndServe(ctx context.Context) error { trustdomainv1.RegisterTrustDomainServer(tcpServer, e.APIServers.TrustDomainServer) trustdomainv1.RegisterTrustDomainServer(udsServer, e.APIServers.TrustDomainServer) - // Register Health and Debug only on UDS server + // UDP only + loggerv1.RegisterLoggerServer(udsServer, e.APIServers.LoggerServer) grpc_health_v1.RegisterHealthServer(udsServer, e.APIServers.HealthServer) debugv1_pb.RegisterDebugServer(udsServer, e.APIServers.DebugServer) From 660ef92210985b6f14c1836d4d94c068481a1cde Mon Sep 17 00:00:00 2001 From: Edwin Buck Date: Fri, 2 Feb 2024 09:59:28 -0600 Subject: [PATCH 02/20] Fixed logical error in comment. Signed-off-by: Edwin Buck --- pkg/common/logger/logger.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/common/logger/logger.go b/pkg/common/logger/logger.go index 15177028d9..0cd056d30c 100644 --- a/pkg/common/logger/logger.go +++ b/pkg/common/logger/logger.go @@ -19,7 +19,7 @@ func sortByName(lesser, greater loggerRecord) bool { var index int // declaration for post loop processing for index = 0; index < len(lesserSegments); index++ { // lesser is "... (index) segment", greater is "... (index)" - // fewer segments first, so lesser is not lesser + // fewer segments in greater, so lesser is not lesser if index >= len(greaterSegments) { return false } @@ -37,7 +37,7 @@ func sortByName(lesser, greater loggerRecord) bool { // continue comparison advancing to next segment } // lesser is "... (index)", greater is "... (index) aaa" - // fewer segements first, so lesser is lesser + // fewer segements in greater, so lesser is lesser return len(lesserSegments) < len(greaterSegments) } From 0d9355aedfa08cfed754efee7a2445929062a1d8 Mon Sep 17 00:00:00 2001 From: Edwin Buck Date: Sun, 11 Feb 2024 19:47:04 -0600 Subject: [PATCH 03/20] Functional for get and set logger commands. Signed-off-by: Edwin Buck --- cmd/spire-server/cli/cli.go | 7 +- cmd/spire-server/cli/logger/get.go | 77 +++++++++++++ cmd/spire-server/cli/logger/listCommand.go | 127 --------------------- cmd/spire-server/cli/logger/set.go | 91 +++++++++++++++ cmd/spire-server/cli/run/run.go | 1 + pkg/common/logger/logger.go | 49 -------- pkg/common/logger/logger_test.go | 42 ------- pkg/server/api/logger/v1/service.go | 60 ++++++++++ pkg/server/authpolicy/policy_data.json | 10 ++ pkg/server/config.go | 2 + pkg/server/endpoints/config.go | 11 +- pkg/server/endpoints/middleware.go | 2 + pkg/server/server.go | 5 + 13 files changed, 263 insertions(+), 221 deletions(-) create mode 100644 cmd/spire-server/cli/logger/get.go delete mode 100644 cmd/spire-server/cli/logger/listCommand.go create mode 100644 cmd/spire-server/cli/logger/set.go delete mode 100644 pkg/common/logger/logger.go delete mode 100644 pkg/common/logger/logger_test.go create mode 100644 pkg/server/api/logger/v1/service.go diff --git a/cmd/spire-server/cli/cli.go b/cmd/spire-server/cli/cli.go index c85372d241..4fdb631cea 100644 --- a/cmd/spire-server/cli/cli.go +++ b/cmd/spire-server/cli/cli.go @@ -97,8 +97,11 @@ func (cc *CLI) Run(ctx context.Context, args []string) int { "federation update": func() (cli.Command, error) { return federation.NewUpdateCommand(), nil }, - "logger list": func() (cli.Command, error) { - return logger.NewListCommand(), nil + "logger get": func() (cli.Command, error) { + return logger.NewGetCommand(), nil + }, + "logger set": func() (cli.Command, error) { + return logger.NewSetCommand(), nil }, "run": func() (cli.Command, error) { return run.NewRunCommand(ctx, cc.LogOptions, cc.AllowUnknownConfig), nil diff --git a/cmd/spire-server/cli/logger/get.go b/cmd/spire-server/cli/logger/get.go new file mode 100644 index 0000000000..0a5b23ad89 --- /dev/null +++ b/cmd/spire-server/cli/logger/get.go @@ -0,0 +1,77 @@ +package logger + +import ( + "context" + "errors" + "flag" + "fmt" + + "github.com/sirupsen/logrus" + "github.com/mitchellh/cli" + api "github.com/spiffe/spire-api-sdk/proto/spire/api/server/logger/v1" + "github.com/spiffe/spire-api-sdk/proto/spire/api/types" + "github.com/spiffe/spire/cmd/spire-server/util" + commoncli "github.com/spiffe/spire/pkg/common/cli" + "github.com/spiffe/spire/pkg/common/cliprinter" +) + +type getCommand struct { + env *commoncli.Env + printer cliprinter.Printer +} + +// Returns a cli.command that gets the logger information using +// the default cli environment. +func NewGetCommand() cli.Command { + return NewGetCommandWithEnv(commoncli.DefaultEnv) +} + +// Returns a cli.command that gets the root logger information. +func NewGetCommandWithEnv(env *commoncli.Env) cli.Command { + return util.AdaptCommand(env, &getCommand{env: env}) +} + +// The name of the command. +func (_ *getCommand) Name() string { + return "logger get" +} + +// The help presented description of the command. +func (_ *getCommand) Synopsis() string { + return "Gets the logger details" +} + +// Adds additional flags specific to the command. +func (c *getCommand) AppendFlags(fs *flag.FlagSet) { + cliprinter.AppendFlagWithCustomPretty(&c.printer, fs, c.env, c.prettyPrintLogger) +} + +// The routine that executes the command +func (c *getCommand) Run(ctx context.Context, _ *commoncli.Env, serverClient util.ServerClient) error { + + logger, err := serverClient.NewLoggerClient().GetLogger(ctx, &api.GetLoggerRequest{}) + if err != nil { + return fmt.Errorf("error fetching logger: %w", err) + } + + return c.printer.PrintProto(logger) +} + +// Formatting for the logger under pretty printing of output. +func (l* getCommand) prettyPrintLogger(env *commoncli.Env, results ...any) error { + logger, ok := results[0].(*types.Logger) + if !ok { + return errors.New("internal error: logger not found; please report this as a bug") + } + if err := env.Printf("Logger Level : %s\n", logrus.Level(logger.CurrentLevel)); err != nil { + return err + } + if err := env.Printf("Logger Default : %s\n", logrus.Level(logger.DefaultLevel)); err != nil { + return err + } + if err := env.Println(); err != nil { + return err + } + return nil +} + diff --git a/cmd/spire-server/cli/logger/listCommand.go b/cmd/spire-server/cli/logger/listCommand.go deleted file mode 100644 index 732cd8ed79..0000000000 --- a/cmd/spire-server/cli/logger/listCommand.go +++ /dev/null @@ -1,127 +0,0 @@ -package logger - -import ( - "context" - "errors" - "flag" - "fmt" - - "github.com/mitchellh/cli" - api "github.com/spiffe/spire-api-sdk/proto/spire/api/server/logger/v1" - "github.com/spiffe/spire-api-sdk/proto/spire/api/types" - "github.com/spiffe/spire/cmd/spire-server/util" - commoncli "github.com/spiffe/spire/pkg/common/cli" - "github.com/spiffe/spire/pkg/common/cliprinter" -) - -const PAGE_SIZE = 500 - -type listCommand struct { - env *commoncli.Env - printer cliprinter.Printer - serverClient util.ServerClient - - requestRoot string - includeSubloggers bool - pageSize int32 -} - -// Returns a cli.command that lists all of the named loggers -func NewListCommand() cli.Command { - return NewListCommandWithEnv(commoncli.DefaultEnv) -} - -func NewListCommandWithEnv(env *commoncli.Env) cli.Command { - return util.AdaptCommand(env, &listCommand{env: env}) -} - -func (_ *listCommand) Name() string { - return "logger list" -} - -func (_ *listCommand) Synopsis() string { - return "Lists loggers and their log levels" -} - -func (c *listCommand) Run(ctx context.Context, _ *commoncli.Env, serverClient util.ServerClient) error { - c.serverClient = serverClient - - var err error // predeclare for post loop error processing - response := &api.ListLoggersResponse{} - for page, err := c.pageRequest(ctx, ""); - err != nil && page.NextPageToken != ""; - page, err = c.pageRequest(ctx, page.NextPageToken) { - response.Loggers = append(response.Loggers, page.Loggers...) - } - if err != nil { - return fmt.Errorf("error fetching loggers: %w", err) - } - - return nil -} - -// Requests just one page. -func (c *listCommand) pageRequest(ctx context.Context, pageToken string) (*api.ListLoggersResponse, error) { - return c.serverClient.NewLoggerClient().ListLoggers(ctx, &api.ListLoggersRequest{ - RootName: c.requestRoot, - WithSubloggers: c.includeSubloggers, - PageSize: c.pageSize, - PageToken: pageToken, - }) -} - -func (c *listCommand) AppendFlags(fs *flag.FlagSet) { - fs.StringVar(&c.requestRoot, "name", "", - "The name of the logger (\"\" for root)") - fs.BoolVar(&c.includeSubloggers, "subloggers", true, - "Include subloggers of \"name\"") - cliprinter.AppendFlagWithCustomPretty(&c.printer, fs, c.env, c.prettyPrintLoggers) -} - -func (l* listCommand) prettyPrintLoggers(env *commoncli.Env, results ...any) error { - response, ok := results[0].(*api.ListLoggersResponse) - if !ok { - return errors.New("internal error: print list cli printer; please report this bug") - } - - count := len(response.Loggers) - switch count { - case 0: - if err := env.Printf("No loggers found\n"); err != nil { - return err - } - case 1: - if err := env.Printf("Found 1 logger\n"); err != nil { - return err - } - default: - if err := env.Printf("Found %d loggers\n", count); err != nil { - return err - } - if err := env.Println(); err != nil { - return err - } - for _, logger := range response.Loggers { - if err := l.printLogger(env, logger); err != nil { - return err - } - } - } - return nil -} - -func (l *listCommand) printLogger(env *commoncli.Env, logger *types.Logger) error { - if err := env.Printf("Logger Name : %s\n", logger.Name); err != nil { - return err - } - if err := env.Printf("Logger Level : %d\n", logger.CurrentLevel); err != nil { - return err - } - if err := env.Printf("Logger Default : %d\n", logger.DefaultLevel); err != nil { - return err - } - if err := env.Println(); err != nil { - return err - } - return nil -} diff --git a/cmd/spire-server/cli/logger/set.go b/cmd/spire-server/cli/logger/set.go new file mode 100644 index 0000000000..47090f91a1 --- /dev/null +++ b/cmd/spire-server/cli/logger/set.go @@ -0,0 +1,91 @@ +package logger + +import ( + "context" + "errors" + "flag" + "fmt" + "strings" + + "github.com/mitchellh/cli" + api "github.com/spiffe/spire-api-sdk/proto/spire/api/server/logger/v1" + "github.com/spiffe/spire-api-sdk/proto/spire/api/types" + "github.com/spiffe/spire/cmd/spire-server/util" + commoncli "github.com/spiffe/spire/pkg/common/cli" + "github.com/spiffe/spire/pkg/common/cliprinter" +) + +type setCommand struct { + env *commoncli.Env + newLevel string + printer cliprinter.Printer +} + +// Returns a cli.command that sets the log level using the default +// cli enviornment. +func NewSetCommand() cli.Command { + return NewSetCommandWithEnv(commoncli.DefaultEnv) +} + +// Returns a cli.command that sets the log level. +func NewSetCommandWithEnv(env *commoncli.Env) cli.Command { + return util.AdaptCommand(env, &setCommand{env: env}) +} + +// The name of the command. +func (_ *setCommand) Name() string { + return "logger set" +} + +// The help presented description of the command. +func (_ *setCommand) Synopsis() string { + return "Sets the logger attributes" +} + +// Adds additional flags specific to the command. +func (c *setCommand) AppendFlags(fs *flag.FlagSet) { + fs.StringVar(&c.newLevel, "level", "", "the new log level, one of (debug)") + cliprinter.AppendFlagWithCustomPretty(&c.printer, fs, c.env, c.prettyPrintLogger) +} + + +// The routine that executes the command +func (c *setCommand) Run(ctx context.Context, _ *commoncli.Env, serverClient util.ServerClient) error { + if c.newLevel != "" { + fmt.Errorf("the newLevel is %s", c.newLevel) + grpc_key := strings.ToUpper(c.newLevel) + "_LEVEL" + value, found := api.SetLogLevelRequest_SetValue_value[grpc_key] + if !found { + return fmt.Errorf("the value %s is not a valid setting", c.newLevel) + } + + logger, err := serverClient.NewLoggerClient().SetLogLevel(ctx, &api.SetLogLevelRequest{ + LogLevel: api.SetLogLevelRequest_SetValue(value), + }) + if err != nil { + return fmt.Errorf("error fetching logger: %w", err) + } + + return c.printer.PrintProto(logger) + } + + return fmt.Errorf("a value must be set") +} + +func (l* setCommand) prettyPrintLogger(env *commoncli.Env, results ...any) error { + logger, ok := results[0].(*types.Logger) + if !ok { + return errors.New("internal error: logger not found; please report this as a bug") + } + if err := env.Printf("Logger Level : %s\n", logger.CurrentLevel); err != nil { + return err + } + if err := env.Printf("Logger Default : %d\n", logger.DefaultLevel); err != nil { + return err + } + if err := env.Println(); err != nil { + return err + } + return nil +} + diff --git a/cmd/spire-server/cli/run/run.go b/cmd/spire-server/cli/run/run.go index 2168f270db..5fc39c274f 100644 --- a/cmd/spire-server/cli/run/run.go +++ b/cmd/spire-server/cli/run/run.go @@ -385,6 +385,7 @@ func NewServerConfig(c *Config, logOptions []log.Option, allowUnknownConfig bool } logger, err := log.NewLogger(logOptions...) + sc.DefaultLogLevel, _ = logrus.ParseLevel(c.Server.LogLevel) if err != nil { return nil, fmt.Errorf("could not start logger: %w", err) } diff --git a/pkg/common/logger/logger.go b/pkg/common/logger/logger.go deleted file mode 100644 index 0cd056d30c..0000000000 --- a/pkg/common/logger/logger.go +++ /dev/null @@ -1,49 +0,0 @@ -package logger - -import ( - "strings" - "sync" - - "github.com/google/btree" -) - -type loggerRecord struct { - name string -} - -// True if a is first, false otherwise -func sortByName(lesser, greater loggerRecord) bool { - lesserSegments := strings.Split(lesser.name, ".") - greaterSegments := strings.Split(greater.name, ".") - - var index int // declaration for post loop processing - for index = 0; index < len(lesserSegments); index++ { - // lesser is "... (index) segment", greater is "... (index)" - // fewer segments in greater, so lesser is not lesser - if index >= len(greaterSegments) { - return false - } - // lesser is "... (index) aaa", greater is "... (index) b" - // lexographic ordering of segments, so lesser is lesser - if lesserSegments[index] < greaterSegments[index] { - return true - } - // lesser is "... (index) b", greater is "... (index) aaa" - // lexographic ordering of segments, so lesser is not lesser - if lesserSegments[index] > greaterSegments[index] { - return false - } - // lesser is "... (index) aaa", greater is "... (index) aaa" - // continue comparison advancing to next segment - } - // lesser is "... (index)", greater is "... (index) aaa" - // fewer segements in greater, so lesser is lesser - return len(lesserSegments) < len(greaterSegments) -} - -type Registry struct { - mu sync.RWMutex - - loggersByName *btree.BTreeG[loggerRecord] -} - diff --git a/pkg/common/logger/logger_test.go b/pkg/common/logger/logger_test.go deleted file mode 100644 index 9963d5d599..0000000000 --- a/pkg/common/logger/logger_test.go +++ /dev/null @@ -1,42 +0,0 @@ -package logger - -import ( - "testing" - - "github.com/stretchr/testify/require" -) - -// Desired order -// "" always first -// a.b before a.b.c -// a.b before a.c -// a.b before b.b -func TestSortByName(t *testing.T) { - var tests = []struct { - First string - Second string - }{ - { "", "a"}, - {"0", "1"}, - {"02", "1"}, - {"1", "a"}, - {"a", "aa"}, - {"a", "ab"}, - {"aa", "ab"}, - {"aa", "abc"}, - {"ab", "abc"}, - {"a.b", "aa.c"}, - {"a.b.c", "aa.a"}, - {"a.b", "a.c"}, - } - for _, testCase := range tests { - first := loggerRecord{ - name: testCase.First, - } - second := loggerRecord{ - name: testCase.Second, - } - require.True(t, sortByName(first, second), "Name %s sould come before %s", first.name, second.name) - require.False(t, sortByName(second, first), "Name %s sould come after %s", second.name, first.name) - } -} diff --git a/pkg/server/api/logger/v1/service.go b/pkg/server/api/logger/v1/service.go new file mode 100644 index 0000000000..02c8128aa2 --- /dev/null +++ b/pkg/server/api/logger/v1/service.go @@ -0,0 +1,60 @@ +package logger + +import ( + "context" + + loggerv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/logger/v1" + "github.com/spiffe/spire-api-sdk/proto/spire/api/types" + + "github.com/sirupsen/logrus" + "google.golang.org/grpc" +) + +type Config struct { + DefaultLevel logrus.Level +} + +type Service struct { + loggerv1.UnsafeLoggerServer + + DefaultLevel logrus.Level +} + +func New(config Config) *Service { + logrus.WithFields(logrus.Fields{ + "DefaultLevel": config.DefaultLevel, + }).Info("logger service Configured") + return &Service{ + DefaultLevel: config.DefaultLevel, + } +} + +func RegisterService(server *grpc.Server, service *Service) { + loggerv1.RegisterLoggerServer(server, service) +} + +func (service *Service) GetLogger(ctx context.Context, req *loggerv1.GetLoggerRequest) (*types.Logger, error) { + logrus.Info("GetLogger Called") + logger := &types.Logger{ + CurrentLevel: types.Logger_LogLevel(logrus.GetLevel()), + DefaultLevel: types.Logger_LogLevel(service.DefaultLevel), + } + return logger, nil +} + +func (service *Service) SetLogLevel(ctx context.Context, req *loggerv1.SetLogLevelRequest) (*types.Logger, error) { + logrus.WithFields(logrus.Fields{ + "RequestLevel": loggerv1.SetLogLevelRequest_SetValue_name[int32(req.LogLevel)], + }).Info("SetLogger Called") + setLevel := loggerv1.SetLogLevelRequest_SetValue(req.LogLevel) + if setLevel == loggerv1.SetLogLevelRequest_DEFAULT_LEVEL { + logrus.SetLevel(service.DefaultLevel) + } else { + logrus.SetLevel(logrus.Level(req.LogLevel)) + } + logger := &types.Logger{ + CurrentLevel: types.Logger_LogLevel(logrus.GetLevel()), + DefaultLevel: types.Logger_LogLevel(service.DefaultLevel), + } + return logger, nil +} diff --git a/pkg/server/authpolicy/policy_data.json b/pkg/server/authpolicy/policy_data.json index 9b3556cf4f..d79b7ddb88 100644 --- a/pkg/server/authpolicy/policy_data.json +++ b/pkg/server/authpolicy/policy_data.json @@ -113,6 +113,16 @@ "full_method": "/spire.api.server.entry.v1.Entry/SyncAuthorizedEntries", "allow_agent": true }, + { + "full_method": "/spire.api.server.logger.v1.Logger/GetLogger", + "allow_admin": true, + "allow_local": true + }, + { + "full_method": "/spire.api.server.logger.v1.Logger/SetLogLevel", + "allow_admin": true, + "allow_local": true + }, { "full_method": "/spire.api.server.agent.v1.Agent/CountAgents", "allow_admin": true, diff --git a/pkg/server/config.go b/pkg/server/config.go index ae2dffc231..776cdf2c8a 100644 --- a/pkg/server/config.go +++ b/pkg/server/config.go @@ -24,6 +24,8 @@ type Config struct { Log logrus.FieldLogger + DefaultLogLevel logrus.Level + // LogReopener facilitates handling a signal to rotate log file. LogReopener func(context.Context) error diff --git a/pkg/server/endpoints/config.go b/pkg/server/endpoints/config.go index 4ae690d856..47f1df3387 100644 --- a/pkg/server/endpoints/config.go +++ b/pkg/server/endpoints/config.go @@ -19,6 +19,7 @@ import ( bundlev1 "github.com/spiffe/spire/pkg/server/api/bundle/v1" debugv1 "github.com/spiffe/spire/pkg/server/api/debug/v1" entryv1 "github.com/spiffe/spire/pkg/server/api/entry/v1" + loggerv1 "github.com/spiffe/spire/pkg/server/api/logger/v1" healthv1 "github.com/spiffe/spire/pkg/server/api/health/v1" svidv1 "github.com/spiffe/spire/pkg/server/api/svid/v1" trustdomainv1 "github.com/spiffe/spire/pkg/server/api/trustdomain/v1" @@ -61,7 +62,12 @@ type Config struct { // Makes policy decisions AuthPolicyEngine *authpolicy.Engine - Log logrus.FieldLogger + // The fielLogger interface + Log logrus.FieldLogger + + // The default (original config) log level + DefaultLogLevel logrus.Level + Metrics telemetry.Metrics // RateLimit holds rate limiting configurations. @@ -157,6 +163,9 @@ func (c *Config) makeAPIServers(entryFetcher api.AuthorizedEntryFetcher) APIServ TrustDomain: c.TrustDomain, DataStore: ds, }), + LoggerServer: loggerv1.New(loggerv1.Config{ + DefaultLevel: c.DefaultLogLevel, + }), SVIDServer: svidv1.New(svidv1.Config{ TrustDomain: c.TrustDomain, EntryFetcher: entryFetcher, diff --git a/pkg/server/endpoints/middleware.go b/pkg/server/endpoints/middleware.go index 38d36ad2c4..a54dd2b846 100644 --- a/pkg/server/endpoints/middleware.go +++ b/pkg/server/endpoints/middleware.go @@ -153,6 +153,8 @@ func RateLimits(config RateLimitConfig) map[string]api.RateLimiter { "/spire.api.server.entry.v1.Entry/BatchDeleteEntry": noLimit, "/spire.api.server.entry.v1.Entry/GetAuthorizedEntries": noLimit, "/spire.api.server.entry.v1.Entry/SyncAuthorizedEntries": noLimit, + "/spire.api.server.logger.v1.Logger/GetLogger": noLimit, + "/spire.api.server.logger.v1.Logger/SetLogLevel": noLimit, "/spire.api.server.agent.v1.Agent/CountAgents": noLimit, "/spire.api.server.agent.v1.Agent/ListAgents": noLimit, "/spire.api.server.agent.v1.Agent/GetAgent": noLimit, diff --git a/pkg/server/server.go b/pkg/server/server.go index 0c0c7a3959..d2d2b86907 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -75,6 +75,10 @@ func (s *Server) run(ctx context.Context) (err error) { telemetry.DataDir: s.config.DataDir, }).Info("Configured") + s.config.Log.WithFields(logrus.Fields{ + "DefaultLogLevel": s.config.DefaultLogLevel, + }).Info("Log Level") + // create the data directory if needed if err := diskutil.CreateDataDirectory(s.config.DataDir); err != nil { return err @@ -386,6 +390,7 @@ func (s *Server) newEndpointsServer(ctx context.Context, catalog catalog.Catalog Catalog: catalog, ServerCA: serverCA, Log: s.config.Log.WithField(telemetry.SubsystemName, telemetry.Endpoints), + DefaultLogLevel: s.config.DefaultLogLevel, Metrics: metrics, JWTKeyPublisher: jwtKeyPublisher, RateLimit: s.config.RateLimit, From adef0161cca420718ab802217df31b24366a39fb Mon Sep 17 00:00:00 2001 From: Edwin Buck Date: Mon, 12 Feb 2024 07:42:08 -0600 Subject: [PATCH 04/20] Adjustments to improve JSON output formatting. JSON now outputs log levels as their logrus lower-case strings. Some unit testing on logger.get cli commands. Signed-off-by: Edwin Buck --- cmd/spire-server/cli/logger/get.go | 4 +- cmd/spire-server/cli/logger/get_posix_test.go | 12 + cmd/spire-server/cli/logger/get_test.go | 235 ++++++++++++++++++ cmd/spire-server/cli/logger/set.go | 7 +- pkg/server/api/logger/v1/service.go | 2 +- 5 files changed, 254 insertions(+), 6 deletions(-) create mode 100644 cmd/spire-server/cli/logger/get_posix_test.go create mode 100644 cmd/spire-server/cli/logger/get_test.go diff --git a/cmd/spire-server/cli/logger/get.go b/cmd/spire-server/cli/logger/get.go index 0a5b23ad89..db22734ea9 100644 --- a/cmd/spire-server/cli/logger/get.go +++ b/cmd/spire-server/cli/logger/get.go @@ -63,10 +63,10 @@ func (l* getCommand) prettyPrintLogger(env *commoncli.Env, results ...any) error if !ok { return errors.New("internal error: logger not found; please report this as a bug") } - if err := env.Printf("Logger Level : %s\n", logrus.Level(logger.CurrentLevel)); err != nil { + if err := env.Printf("Logger Level : %s\n", logrus.Level(logger.CurrentLevel)); err != nil { return err } - if err := env.Printf("Logger Default : %s\n", logrus.Level(logger.DefaultLevel)); err != nil { + if err := env.Printf("Logger Default: %s\n", logrus.Level(logger.DefaultLevel)); err != nil { return err } if err := env.Println(); err != nil { diff --git a/cmd/spire-server/cli/logger/get_posix_test.go b/cmd/spire-server/cli/logger/get_posix_test.go new file mode 100644 index 0000000000..21fb0e666e --- /dev/null +++ b/cmd/spire-server/cli/logger/get_posix_test.go @@ -0,0 +1,12 @@ +//go:build !windows +package logger_test + +var ( + getUsage = `Usage of logger get: + -output value + Desired output format (pretty, json); default: pretty. + -socketPath string + Path to the SPIRE Server API socket (default "/tmp/spire-server/private/api.sock") +` +) + diff --git a/cmd/spire-server/cli/logger/get_test.go b/cmd/spire-server/cli/logger/get_test.go new file mode 100644 index 0000000000..9c7f4fae28 --- /dev/null +++ b/cmd/spire-server/cli/logger/get_test.go @@ -0,0 +1,235 @@ +package logger_test + +import ( + "context" + "testing" + "github.com/stretchr/testify/require" + "github.com/spiffe/spire/test/spiretest" + + "bytes" + + "github.com/mitchellh/cli" + "github.com/spiffe/spire/cmd/spire-server/cli/common" + "github.com/spiffe/spire/cmd/spire-server/cli/logger" + "github.com/spiffe/spire-api-sdk/proto/spire/api/types" + loggerv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/logger/v1" + commoncli "github.com/spiffe/spire/pkg/common/cli" + + "google.golang.org/grpc" +) + +var availableFormats = []string{"pretty", "json"} + +type loggerTest struct { + stdin *bytes.Buffer + stdout *bytes.Buffer + stderr *bytes.Buffer + args []string + server *fakeLoggerServer + client cli.Command +} + +func (l *loggerTest) afterTest(t *testing.T) { + t.Logf("TEST:%s", t.Name()) + t.Logf("STDOUT:\n%s", l.stdout.String()) + t.Logf("STDIN:\n%s", l.stdin.String()) + t.Logf("STDERR:\n%s", l.stderr.String()) +} + +func TestGetHelp(t *testing.T) { + for _, tt := range []struct{ + currentLevel types.Logger_LogLevel + defaultLevel types.Logger_LogLevel + }{ + { + currentLevel: types.Logger_debug, + defaultLevel: types.Logger_warn, + }, + { + currentLevel: types.Logger_panic, + defaultLevel: types.Logger_trace, + }, + { + currentLevel: types.Logger_fatal, + defaultLevel: types.Logger_error, + }, + { + currentLevel: types.Logger_info, + defaultLevel: types.Logger_info, + }, + }{ + server := &fakeLoggerServer{ + currentLevel: tt.currentLevel, + defaultLevel: tt.defaultLevel, + } + test := setupTest(t, server, logger.NewGetCommandWithEnv) + test.client.Help() + + require.Equal(t, getUsage, test.stderr.String()) + } +} + +func TestGetSynopsis(t *testing.T) { + cmd := logger.NewGetCommand() + require.Equal(t, "Gets the logger details", cmd.Synopsis()) +} + +func TestGet(t *testing.T) { + for _, tt := range []struct{ + name string + args []string + currentLevel types.Logger_LogLevel + defaultLevel types.Logger_LogLevel + expectReturnCode int + expectStdout string + expectStderr string + serverErr error + + }{ + { + name: "configured to info, set to info, using pretty output", + args: []string{"-output", "pretty"}, + currentLevel: types.Logger_info, + defaultLevel: types.Logger_info, + expectReturnCode: 0, + expectStdout: `Logger Level : info +Logger Default: info + +`, + }, + { + name: "configured to debug, set to warn, using pretty output", + args: []string{"-output", "pretty"}, + currentLevel: types.Logger_warn, + defaultLevel: types.Logger_debug, + expectReturnCode: 0, + expectStdout: `Logger Level : warning +Logger Default: debug + +`, + }, + { + name: "configured to error, set to trace, using pretty output", + args: []string{"-output", "pretty"}, + currentLevel: types.Logger_trace, + defaultLevel: types.Logger_error, + expectReturnCode: 0, + expectStdout: `Logger Level : trace +Logger Default: error + +`, + }, + { + name: "configured to panic, set to fatal, using pretty output", + args: []string{"-output", "pretty"}, + currentLevel: types.Logger_fatal, + defaultLevel: types.Logger_panic, + expectReturnCode: 0, + expectStdout: `Logger Level : fatal +Logger Default: panic + +`, + }, + { + name: "configured to info, set to info, using json output", + args: []string{"-output", "json"}, + currentLevel: types.Logger_info, + defaultLevel: types.Logger_info, + expectReturnCode: 0, + expectStdout: `{"current_level":"info","default_level":"info"} +`, + }, + { + name: "configured to debug, set to warn, using json output", + args: []string{"-output", "json"}, + currentLevel: types.Logger_warn, + defaultLevel: types.Logger_debug, + expectReturnCode: 0, + expectStdout: `{"current_level":"warn","default_level":"debug"} +`, + }, + { + name: "configured to error, set to trace, using json output", + args: []string{"-output", "json"}, + currentLevel: types.Logger_trace, + defaultLevel: types.Logger_error, + expectReturnCode: 0, + expectStdout: `{"current_level":"trace","default_level":"error"} +`, + }, + { + name: "configured to panic, set to fatal, using json output", + args: []string{"-output", "json"}, + currentLevel: types.Logger_fatal, + defaultLevel: types.Logger_panic, + expectReturnCode: 0, + expectStdout: `{"current_level":"fatal","default_level":"panic"} +`, + }, + } { + t.Run(tt.name, func(t *testing.T) { + server := &fakeLoggerServer{ + currentLevel: tt.currentLevel, + defaultLevel: tt.defaultLevel, + } + test := setupTest(t, server, logger.NewGetCommandWithEnv) + test.server.err = tt.serverErr + args := tt.args + returnCode := test.client.Run(append(test.args, args...)) + require.Equal(t, tt.expectStdout, test.stdout.String()) + require.Equal(t, tt.expectStderr, test.stderr.String()) + require.Equal(t, tt.expectReturnCode, returnCode) + }) + } +} + +func setupTest(t *testing.T, server *fakeLoggerServer, newClient func(*commoncli.Env) cli.Command) *loggerTest { + addr := spiretest.StartGRPCServer(t, func(s *grpc.Server) { + loggerv1.RegisterLoggerServer(s, server) + }) + + stdin := new(bytes.Buffer) + stdout := new(bytes.Buffer) + stderr := new(bytes.Buffer) + + client := newClient(&commoncli.Env{ + Stdin: stdin, + Stdout: stdout, + Stderr: stderr, + }) + + test := &loggerTest{ + stdin: stdin, + stdout: stdout, + stderr: stderr, + args: []string{common.AddrArg, common.GetAddr(addr)}, + server: server, + client: client, + } + + t.Cleanup(func() { + test.afterTest(t) + }) + + return test +} + +type fakeLoggerServer struct { + loggerv1.UnimplementedLoggerServer + + currentLevel types.Logger_LogLevel + defaultLevel types.Logger_LogLevel + + err error +} + +func (s *fakeLoggerServer) GetLogger(_ context.Context, _ *loggerv1.GetLoggerRequest) (*types.Logger, error) { + if s.err != nil { + return nil, s.err + } + return &types.Logger{ + CurrentLevel: s.currentLevel, + DefaultLevel: s.defaultLevel, + }, s.err +} + diff --git a/cmd/spire-server/cli/logger/set.go b/cmd/spire-server/cli/logger/set.go index 47090f91a1..f514f3e214 100644 --- a/cmd/spire-server/cli/logger/set.go +++ b/cmd/spire-server/cli/logger/set.go @@ -7,6 +7,7 @@ import ( "fmt" "strings" + "github.com/sirupsen/logrus" "github.com/mitchellh/cli" api "github.com/spiffe/spire-api-sdk/proto/spire/api/server/logger/v1" "github.com/spiffe/spire-api-sdk/proto/spire/api/types" @@ -53,7 +54,7 @@ func (c *setCommand) AppendFlags(fs *flag.FlagSet) { func (c *setCommand) Run(ctx context.Context, _ *commoncli.Env, serverClient util.ServerClient) error { if c.newLevel != "" { fmt.Errorf("the newLevel is %s", c.newLevel) - grpc_key := strings.ToUpper(c.newLevel) + "_LEVEL" + grpc_key := strings.ToUpper(c.newLevel) value, found := api.SetLogLevelRequest_SetValue_value[grpc_key] if !found { return fmt.Errorf("the value %s is not a valid setting", c.newLevel) @@ -77,10 +78,10 @@ func (l* setCommand) prettyPrintLogger(env *commoncli.Env, results ...any) error if !ok { return errors.New("internal error: logger not found; please report this as a bug") } - if err := env.Printf("Logger Level : %s\n", logger.CurrentLevel); err != nil { + if err := env.Printf("Logger Level : %s\n", logrus.Level(logger.CurrentLevel)); err != nil { return err } - if err := env.Printf("Logger Default : %d\n", logger.DefaultLevel); err != nil { + if err := env.Printf("Logger Default : %s\n", logrus.Level(logger.DefaultLevel)); err != nil { return err } if err := env.Println(); err != nil { diff --git a/pkg/server/api/logger/v1/service.go b/pkg/server/api/logger/v1/service.go index 02c8128aa2..4d37c45f21 100644 --- a/pkg/server/api/logger/v1/service.go +++ b/pkg/server/api/logger/v1/service.go @@ -47,7 +47,7 @@ func (service *Service) SetLogLevel(ctx context.Context, req *loggerv1.SetLogLev "RequestLevel": loggerv1.SetLogLevelRequest_SetValue_name[int32(req.LogLevel)], }).Info("SetLogger Called") setLevel := loggerv1.SetLogLevelRequest_SetValue(req.LogLevel) - if setLevel == loggerv1.SetLogLevelRequest_DEFAULT_LEVEL { + if setLevel == loggerv1.SetLogLevelRequest_DEFAULT { logrus.SetLevel(service.DefaultLevel) } else { logrus.SetLevel(logrus.Level(req.LogLevel)) From 57f3da0797b52544e9d1c3bf3375bbd04184ff84 Mon Sep 17 00:00:00 2001 From: Edwin Buck Date: Mon, 19 Feb 2024 04:43:21 -0600 Subject: [PATCH 05/20] Realign to final spire-api-sdk api. Complete unit testing for spire-server cli. Signed-off-by: Edwin Buck --- cmd/spire-server/cli/logger/get.go | 18 +- cmd/spire-server/cli/logger/get_posix_test.go | 12 - cmd/spire-server/cli/logger/get_test.go | 223 +++++++----------- cmd/spire-server/cli/logger/mocks_test.go | 105 +++++++++ cmd/spire-server/cli/logger/printers.go | 20 ++ cmd/spire-server/cli/logger/printers_test.go | 68 ++++++ cmd/spire-server/cli/logger/set.go | 27 +-- cmd/spire-server/cli/logger/set_test.go | 174 ++++++++++++++ pkg/server/api/logger/v1/service.go | 6 +- 9 files changed, 463 insertions(+), 190 deletions(-) delete mode 100644 cmd/spire-server/cli/logger/get_posix_test.go create mode 100644 cmd/spire-server/cli/logger/mocks_test.go create mode 100644 cmd/spire-server/cli/logger/printers.go create mode 100644 cmd/spire-server/cli/logger/printers_test.go create mode 100644 cmd/spire-server/cli/logger/set_test.go diff --git a/cmd/spire-server/cli/logger/get.go b/cmd/spire-server/cli/logger/get.go index db22734ea9..6c359328ee 100644 --- a/cmd/spire-server/cli/logger/get.go +++ b/cmd/spire-server/cli/logger/get.go @@ -2,14 +2,11 @@ package logger import ( "context" - "errors" "flag" "fmt" - "github.com/sirupsen/logrus" "github.com/mitchellh/cli" api "github.com/spiffe/spire-api-sdk/proto/spire/api/server/logger/v1" - "github.com/spiffe/spire-api-sdk/proto/spire/api/types" "github.com/spiffe/spire/cmd/spire-server/util" commoncli "github.com/spiffe/spire/pkg/common/cli" "github.com/spiffe/spire/pkg/common/cliprinter" @@ -59,19 +56,6 @@ func (c *getCommand) Run(ctx context.Context, _ *commoncli.Env, serverClient uti // Formatting for the logger under pretty printing of output. func (l* getCommand) prettyPrintLogger(env *commoncli.Env, results ...any) error { - logger, ok := results[0].(*types.Logger) - if !ok { - return errors.New("internal error: logger not found; please report this as a bug") - } - if err := env.Printf("Logger Level : %s\n", logrus.Level(logger.CurrentLevel)); err != nil { - return err - } - if err := env.Printf("Logger Default: %s\n", logrus.Level(logger.DefaultLevel)); err != nil { - return err - } - if err := env.Println(); err != nil { - return err - } - return nil + return PrettyPrintLogger(env, results...) } diff --git a/cmd/spire-server/cli/logger/get_posix_test.go b/cmd/spire-server/cli/logger/get_posix_test.go deleted file mode 100644 index 21fb0e666e..0000000000 --- a/cmd/spire-server/cli/logger/get_posix_test.go +++ /dev/null @@ -1,12 +0,0 @@ -//go:build !windows -package logger_test - -var ( - getUsage = `Usage of logger get: - -output value - Desired output format (pretty, json); default: pretty. - -socketPath string - Path to the SPIRE Server API socket (default "/tmp/spire-server/private/api.sock") -` -) - diff --git a/cmd/spire-server/cli/logger/get_test.go b/cmd/spire-server/cli/logger/get_test.go index 9c7f4fae28..e562ae78a8 100644 --- a/cmd/spire-server/cli/logger/get_test.go +++ b/cmd/spire-server/cli/logger/get_test.go @@ -1,72 +1,28 @@ package logger_test import ( - "context" + "errors" "testing" "github.com/stretchr/testify/require" - "github.com/spiffe/spire/test/spiretest" - "bytes" - - "github.com/mitchellh/cli" - "github.com/spiffe/spire/cmd/spire-server/cli/common" "github.com/spiffe/spire/cmd/spire-server/cli/logger" "github.com/spiffe/spire-api-sdk/proto/spire/api/types" - loggerv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/logger/v1" - commoncli "github.com/spiffe/spire/pkg/common/cli" - - "google.golang.org/grpc" ) -var availableFormats = []string{"pretty", "json"} - -type loggerTest struct { - stdin *bytes.Buffer - stdout *bytes.Buffer - stderr *bytes.Buffer - args []string - server *fakeLoggerServer - client cli.Command -} - -func (l *loggerTest) afterTest(t *testing.T) { - t.Logf("TEST:%s", t.Name()) - t.Logf("STDOUT:\n%s", l.stdout.String()) - t.Logf("STDIN:\n%s", l.stdin.String()) - t.Logf("STDERR:\n%s", l.stderr.String()) -} +var ( + getUsage = `Usage of logger get: + -output value + Desired output format (pretty, json); default: pretty. + -socketPath string + Path to the SPIRE Server API socket (default "/tmp/spire-server/private/api.sock") +` +) func TestGetHelp(t *testing.T) { - for _, tt := range []struct{ - currentLevel types.Logger_LogLevel - defaultLevel types.Logger_LogLevel - }{ - { - currentLevel: types.Logger_debug, - defaultLevel: types.Logger_warn, - }, - { - currentLevel: types.Logger_panic, - defaultLevel: types.Logger_trace, - }, - { - currentLevel: types.Logger_fatal, - defaultLevel: types.Logger_error, - }, - { - currentLevel: types.Logger_info, - defaultLevel: types.Logger_info, - }, - }{ - server := &fakeLoggerServer{ - currentLevel: tt.currentLevel, - defaultLevel: tt.defaultLevel, - } - test := setupTest(t, server, logger.NewGetCommandWithEnv) - test.client.Help() - - require.Equal(t, getUsage, test.stderr.String()) - } + test := setupCliTest(t, nil, logger.NewGetCommandWithEnv) + test.client.Help() + require.Equal(t, "", test.stdout.String()) + require.Equal(t, getUsage, test.stderr.String()) } func TestGetSynopsis(t *testing.T) { @@ -77,20 +33,24 @@ func TestGetSynopsis(t *testing.T) { func TestGet(t *testing.T) { for _, tt := range []struct{ name string + // server state + server *mockLoggerServer + // input args []string - currentLevel types.Logger_LogLevel - defaultLevel types.Logger_LogLevel + // expected items expectReturnCode int expectStdout string expectStderr string - serverErr error - }{ { name: "configured to info, set to info, using pretty output", args: []string{"-output", "pretty"}, - currentLevel: types.Logger_info, - defaultLevel: types.Logger_info, + server: &mockLoggerServer{ + returnLogger: &types.Logger{ + CurrentLevel: types.Logger_info, + DefaultLevel: types.Logger_info, + }, + }, expectReturnCode: 0, expectStdout: `Logger Level : info Logger Default: info @@ -100,8 +60,12 @@ Logger Default: info { name: "configured to debug, set to warn, using pretty output", args: []string{"-output", "pretty"}, - currentLevel: types.Logger_warn, - defaultLevel: types.Logger_debug, + server: &mockLoggerServer{ + returnLogger: &types.Logger{ + CurrentLevel: types.Logger_warn, + DefaultLevel: types.Logger_debug, + }, + }, expectReturnCode: 0, expectStdout: `Logger Level : warning Logger Default: debug @@ -111,8 +75,12 @@ Logger Default: debug { name: "configured to error, set to trace, using pretty output", args: []string{"-output", "pretty"}, - currentLevel: types.Logger_trace, - defaultLevel: types.Logger_error, + server: &mockLoggerServer{ + returnLogger: &types.Logger{ + CurrentLevel: types.Logger_trace, + DefaultLevel: types.Logger_error, + }, + }, expectReturnCode: 0, expectStdout: `Logger Level : trace Logger Default: error @@ -122,8 +90,12 @@ Logger Default: error { name: "configured to panic, set to fatal, using pretty output", args: []string{"-output", "pretty"}, - currentLevel: types.Logger_fatal, - defaultLevel: types.Logger_panic, + server: &mockLoggerServer{ + returnLogger: &types.Logger{ + CurrentLevel: types.Logger_fatal, + DefaultLevel: types.Logger_panic, + }, + }, expectReturnCode: 0, expectStdout: `Logger Level : fatal Logger Default: panic @@ -133,8 +105,12 @@ Logger Default: panic { name: "configured to info, set to info, using json output", args: []string{"-output", "json"}, - currentLevel: types.Logger_info, - defaultLevel: types.Logger_info, + server: &mockLoggerServer{ + returnLogger: &types.Logger{ + CurrentLevel: types.Logger_info, + DefaultLevel: types.Logger_info, + }, + }, expectReturnCode: 0, expectStdout: `{"current_level":"info","default_level":"info"} `, @@ -142,8 +118,12 @@ Logger Default: panic { name: "configured to debug, set to warn, using json output", args: []string{"-output", "json"}, - currentLevel: types.Logger_warn, - defaultLevel: types.Logger_debug, + server: &mockLoggerServer{ + returnLogger: &types.Logger{ + CurrentLevel: types.Logger_warn, + DefaultLevel: types.Logger_debug, + }, + }, expectReturnCode: 0, expectStdout: `{"current_level":"warn","default_level":"debug"} `, @@ -151,8 +131,12 @@ Logger Default: panic { name: "configured to error, set to trace, using json output", args: []string{"-output", "json"}, - currentLevel: types.Logger_trace, - defaultLevel: types.Logger_error, + server: &mockLoggerServer{ + returnLogger: &types.Logger{ + CurrentLevel: types.Logger_trace, + DefaultLevel: types.Logger_error, + }, + }, expectReturnCode: 0, expectStdout: `{"current_level":"trace","default_level":"error"} `, @@ -160,76 +144,43 @@ Logger Default: panic { name: "configured to panic, set to fatal, using json output", args: []string{"-output", "json"}, - currentLevel: types.Logger_fatal, - defaultLevel: types.Logger_panic, + server: &mockLoggerServer{ + returnLogger: &types.Logger{ + CurrentLevel: types.Logger_fatal, + DefaultLevel: types.Logger_panic, + }, + }, expectReturnCode: 0, expectStdout: `{"current_level":"fatal","default_level":"panic"} +`, + }, + { + name: "configured to info, set to info, server will error", + args: []string{"-output", "pretty"}, + server: &mockLoggerServer{ + returnErr: errors.New("server is unavailable"), + }, + expectReturnCode: 1, + expectStderr: `Error: error fetching logger: rpc error: code = Unknown desc = server is unavailable +`, + }, + { + name: "bizzarro world, returns neither logger nor error", + args: []string{"-output", "pretty"}, + server: &mockLoggerServer{ + returnLogger: nil, + }, + expectReturnCode: 1, + expectStderr: `Error: error fetching logger: rpc error: code = Internal desc = grpc: error while marshaling: proto: Marshal called with nil `, }, } { t.Run(tt.name, func(t *testing.T) { - server := &fakeLoggerServer{ - currentLevel: tt.currentLevel, - defaultLevel: tt.defaultLevel, - } - test := setupTest(t, server, logger.NewGetCommandWithEnv) - test.server.err = tt.serverErr - args := tt.args - returnCode := test.client.Run(append(test.args, args...)) + test := setupCliTest(t, tt.server, logger.NewGetCommandWithEnv) + returnCode := test.client.Run(append(test.args, tt.args...)) require.Equal(t, tt.expectStdout, test.stdout.String()) require.Equal(t, tt.expectStderr, test.stderr.String()) require.Equal(t, tt.expectReturnCode, returnCode) }) } } - -func setupTest(t *testing.T, server *fakeLoggerServer, newClient func(*commoncli.Env) cli.Command) *loggerTest { - addr := spiretest.StartGRPCServer(t, func(s *grpc.Server) { - loggerv1.RegisterLoggerServer(s, server) - }) - - stdin := new(bytes.Buffer) - stdout := new(bytes.Buffer) - stderr := new(bytes.Buffer) - - client := newClient(&commoncli.Env{ - Stdin: stdin, - Stdout: stdout, - Stderr: stderr, - }) - - test := &loggerTest{ - stdin: stdin, - stdout: stdout, - stderr: stderr, - args: []string{common.AddrArg, common.GetAddr(addr)}, - server: server, - client: client, - } - - t.Cleanup(func() { - test.afterTest(t) - }) - - return test -} - -type fakeLoggerServer struct { - loggerv1.UnimplementedLoggerServer - - currentLevel types.Logger_LogLevel - defaultLevel types.Logger_LogLevel - - err error -} - -func (s *fakeLoggerServer) GetLogger(_ context.Context, _ *loggerv1.GetLoggerRequest) (*types.Logger, error) { - if s.err != nil { - return nil, s.err - } - return &types.Logger{ - CurrentLevel: s.currentLevel, - DefaultLevel: s.defaultLevel, - }, s.err -} - diff --git a/cmd/spire-server/cli/logger/mocks_test.go b/cmd/spire-server/cli/logger/mocks_test.go new file mode 100644 index 0000000000..ff67c9ba4e --- /dev/null +++ b/cmd/spire-server/cli/logger/mocks_test.go @@ -0,0 +1,105 @@ +package logger_test + +import ( + "io" + "testing" + "github.com/spiffe/spire/test/spiretest" + + "bytes" + "context" + + "github.com/mitchellh/cli" + "github.com/spiffe/spire/cmd/spire-server/cli/common" + commoncli "github.com/spiffe/spire/pkg/common/cli" + "google.golang.org/grpc" + "github.com/spiffe/spire-api-sdk/proto/spire/api/types" + loggerv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/logger/v1" +) + +// an input/output capture struct +type loggerTest struct { + stdin *bytes.Buffer + stdout *bytes.Buffer + stderr *bytes.Buffer + args []string + server *mockLoggerServer + client cli.Command +} + +// serialization of capture +func (l *loggerTest) afterTest(t *testing.T) { + t.Logf("TEST:%s", t.Name()) + t.Logf("STDOUT:\n%s", l.stdout.String()) + t.Logf("STDIN:\n%s", l.stdin.String()) + t.Logf("STDERR:\n%s", l.stderr.String()) +} + +// setup of input/output capture +func setupCliTest(t *testing.T, server *mockLoggerServer, newClient func(*commoncli.Env) cli.Command) *loggerTest { + addr := spiretest.StartGRPCServer(t, func(s *grpc.Server) { + loggerv1.RegisterLoggerServer(s, server) + }) + + stdin := new(bytes.Buffer) + stdout := new(bytes.Buffer) + stderr := new(bytes.Buffer) + + client := newClient(&commoncli.Env{ + Stdin: stdin, + Stdout: stdout, + Stderr: stderr, + }) + + test := &loggerTest{ + stdin: stdin, + stdout: stdout, + stderr: stderr, + args: []string{common.AddrArg, common.GetAddr(addr)}, + server: server, + client: client, + } + + t.Cleanup(func() { + test.afterTest(t) + }) + + return test +} + +// a mock grpc logger server +type mockLoggerServer struct { + loggerv1.UnimplementedLoggerServer + + receivedSetValue loggerv1.SetLogLevelRequest_SetValue + returnLogger *types.Logger + returnErr error +} + +// mock implementation for GetLogger +func (s *mockLoggerServer) GetLogger(_ context.Context, _ *loggerv1.GetLoggerRequest) (*types.Logger, error) { + return s.returnLogger, s.returnErr +} + +func (s *mockLoggerServer) SetLogLevel(_ context.Context, req *loggerv1.SetLogLevelRequest) (*types.Logger, error) { + s.receivedSetValue = req.SetLevel + return s.returnLogger, s.returnErr +} + +var _ io.Writer = &errorWriter{} + +type errorWriter struct { + ReturnError error + Buffer bytes.Buffer +} + +func (e *errorWriter) Write(p []byte) (n int, err error) { + if e.ReturnError != nil { + return 0, e.ReturnError + } + return e.Buffer.Write(p) +} + +func (e *errorWriter) String() string { + return e.Buffer.String() +} + diff --git a/cmd/spire-server/cli/logger/printers.go b/cmd/spire-server/cli/logger/printers.go new file mode 100644 index 0000000000..73b5c4c8a7 --- /dev/null +++ b/cmd/spire-server/cli/logger/printers.go @@ -0,0 +1,20 @@ +package logger + +import ( + "errors" + "github.com/sirupsen/logrus" + "github.com/spiffe/spire-api-sdk/proto/spire/api/types" + commoncli "github.com/spiffe/spire/pkg/common/cli" +) + +func PrettyPrintLogger(env *commoncli.Env, results ...any) error { + logger, ok := results[0].(*types.Logger) + if !ok { + return errors.New("internal error: logger not found; please report this as a bug") + } + if err := env.Printf("Logger Level : %s\nLogger Default: %s\n\n", logrus.Level(logger.CurrentLevel), logrus.Level(logger.DefaultLevel)); err != nil { + return err + } + return nil +} + diff --git a/cmd/spire-server/cli/logger/printers_test.go b/cmd/spire-server/cli/logger/printers_test.go new file mode 100644 index 0000000000..8222141dc8 --- /dev/null +++ b/cmd/spire-server/cli/logger/printers_test.go @@ -0,0 +1,68 @@ +package logger_test + +import ( + "errors" + "testing" + "github.com/stretchr/testify/require" + + "github.com/spiffe/spire/cmd/spire-server/cli/logger" + "github.com/spiffe/spire-api-sdk/proto/spire/api/types" + commoncli "github.com/spiffe/spire/pkg/common/cli" +) + +func TestPrettyPrintLogger(t *testing.T) { + for _, tt := range []struct { + name string + logger interface{} + outWriter errorWriter + errWriter errorWriter + env *commoncli.Env + expectedStdout string + expectedStderr string + expectedError error + }{ + { + name: "test", + logger: &types.Logger{ + CurrentLevel: types.Logger_debug, + DefaultLevel: types.Logger_info, + }, + expectedStdout: `Logger Level : debug +Logger Default: info + +`, + }, + { + name: "test env returning an error", + outWriter: errorWriter{ + ReturnError: errors.New("cannot write"), + }, + logger: &types.Logger{ + CurrentLevel: types.Logger_debug, + DefaultLevel: types.Logger_info, + }, + expectedError: errors.New("cannot write"), + }, + { + name: "test nil logger", + outWriter: errorWriter{ + ReturnError: errors.New("cannot write"), + }, + logger: &types.Entry{ + }, + expectedError: errors.New("internal error: logger not found; please report this as a bug"), + }, + }{ + t.Run(tt.name, func(t *testing.T) { + tt.env = &commoncli.Env{ + Stdout: &tt.outWriter, + Stderr: &tt.errWriter, + } + require.Equal(t, logger.PrettyPrintLogger(tt.env, tt.logger), tt.expectedError) + require.Equal(t, tt.outWriter.String(), tt.expectedStdout) + require.Equal(t, tt.errWriter.String(), tt.expectedStderr) + }) + } + +} + diff --git a/cmd/spire-server/cli/logger/set.go b/cmd/spire-server/cli/logger/set.go index f514f3e214..63ec16448a 100644 --- a/cmd/spire-server/cli/logger/set.go +++ b/cmd/spire-server/cli/logger/set.go @@ -2,15 +2,12 @@ package logger import ( "context" - "errors" "flag" "fmt" "strings" - "github.com/sirupsen/logrus" "github.com/mitchellh/cli" api "github.com/spiffe/spire-api-sdk/proto/spire/api/server/logger/v1" - "github.com/spiffe/spire-api-sdk/proto/spire/api/types" "github.com/spiffe/spire/cmd/spire-server/util" commoncli "github.com/spiffe/spire/pkg/common/cli" "github.com/spiffe/spire/pkg/common/cliprinter" @@ -40,12 +37,12 @@ func (_ *setCommand) Name() string { // The help presented description of the command. func (_ *setCommand) Synopsis() string { - return "Sets the logger attributes" + return "Sets the logger details" } // Adds additional flags specific to the command. func (c *setCommand) AppendFlags(fs *flag.FlagSet) { - fs.StringVar(&c.newLevel, "level", "", "the new log level, one of (debug)") + fs.StringVar(&c.newLevel, "level", "", "The new log level, one of (panic, fatal, error, warn, info, debug, trace, default)") cliprinter.AppendFlagWithCustomPretty(&c.printer, fs, c.env, c.prettyPrintLogger) } @@ -61,7 +58,7 @@ func (c *setCommand) Run(ctx context.Context, _ *commoncli.Env, serverClient uti } logger, err := serverClient.NewLoggerClient().SetLogLevel(ctx, &api.SetLogLevelRequest{ - LogLevel: api.SetLogLevelRequest_SetValue(value), + SetLevel: api.SetLogLevelRequest_SetValue(value), }) if err != nil { return fmt.Errorf("error fetching logger: %w", err) @@ -70,23 +67,9 @@ func (c *setCommand) Run(ctx context.Context, _ *commoncli.Env, serverClient uti return c.printer.PrintProto(logger) } - return fmt.Errorf("a value must be set") + return fmt.Errorf("a value (-level) must be set") } func (l* setCommand) prettyPrintLogger(env *commoncli.Env, results ...any) error { - logger, ok := results[0].(*types.Logger) - if !ok { - return errors.New("internal error: logger not found; please report this as a bug") - } - if err := env.Printf("Logger Level : %s\n", logrus.Level(logger.CurrentLevel)); err != nil { - return err - } - if err := env.Printf("Logger Default : %s\n", logrus.Level(logger.DefaultLevel)); err != nil { - return err - } - if err := env.Println(); err != nil { - return err - } - return nil + return PrettyPrintLogger(env, results...) } - diff --git a/cmd/spire-server/cli/logger/set_test.go b/cmd/spire-server/cli/logger/set_test.go new file mode 100644 index 0000000000..4a6227961b --- /dev/null +++ b/cmd/spire-server/cli/logger/set_test.go @@ -0,0 +1,174 @@ +package logger_test + +import ( + "testing" + "github.com/stretchr/testify/require" + + "github.com/spiffe/spire/cmd/spire-server/cli/logger" + loggerv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/logger/v1" + "github.com/spiffe/spire-api-sdk/proto/spire/api/types" +) + +var ( + setUsage = `Usage of logger set: + -level string + The new log level, one of (panic, fatal, error, warn, info, debug, trace, default) + -output value + Desired output format (pretty, json); default: pretty. + -socketPath string + Path to the SPIRE Server API socket (default "/tmp/spire-server/private/api.sock") +` +) + +func TestSetHelp(t *testing.T) { + test := setupCliTest(t, nil, logger.NewSetCommandWithEnv) + test.client.Help() + require.Equal(t, "", test.stdout.String()) + require.Equal(t, setUsage, test.stderr.String()) +} + +func TestSetSynopsis(t *testing.T) { + cmd := logger.NewSetCommand() + require.Equal(t, "Sets the logger details", cmd.Synopsis()) +} + +func TestSet(t *testing.T) { + for _, tt := range []struct{ + name string + // server state + server *mockLoggerServer + // input + args []string + // expected items + expectedSetValue loggerv1.SetLogLevelRequest_SetValue + expectReturnCode int + expectStdout string + expectStderr string + }{ + { + name: "set to debug, configured to info, using pretty output", + args: []string{"-level", "debug", "-output", "pretty"}, + server: &mockLoggerServer{ + returnLogger: &types.Logger{ + CurrentLevel: types.Logger_debug, + DefaultLevel: types.Logger_info, + }, + }, + expectedSetValue: loggerv1.SetLogLevelRequest_DEBUG, + expectReturnCode: 0, + expectStdout: `Logger Level : debug +Logger Default: info + +`, + }, + { + name: "set to warn, configured to debug, using pretty output", + args: []string{"-level", "warn", "-output", "pretty"}, + server: &mockLoggerServer{ + returnLogger: &types.Logger{ + CurrentLevel: types.Logger_warn, + DefaultLevel: types.Logger_debug, + }, + }, + expectedSetValue: loggerv1.SetLogLevelRequest_WARN, + expectReturnCode: 0, + expectStdout: `Logger Level : warning +Logger Default: debug + +`, + }, + { + name: "set to default, configured to error, using pretty output", + args: []string{"-level", "default", "-output", "pretty"}, + server: &mockLoggerServer{ + returnLogger: &types.Logger{ + CurrentLevel: types.Logger_error, + DefaultLevel: types.Logger_error, + }, + }, + expectedSetValue: loggerv1.SetLogLevelRequest_DEFAULT, + expectReturnCode: 0, + expectStdout: `Logger Level : error +Logger Default: error + +`, + }, + { + name: "set to panic, configured to fatal, using pretty output", + args: []string{"-level", "panic", "-output", "pretty"}, + server: &mockLoggerServer{ + returnLogger: &types.Logger{ + CurrentLevel: types.Logger_panic, + DefaultLevel: types.Logger_fatal, + }, + }, + expectedSetValue: loggerv1.SetLogLevelRequest_PANIC, + expectReturnCode: 0, + expectStdout: `Logger Level : panic +Logger Default: fatal + +`, + }, + { + name: "set with invalid setting of never, logger unadjusted from (info,info)", + args: []string{"-level", "never", "-output", "pretty"}, + server: &mockLoggerServer{ + returnLogger: &types.Logger{ + CurrentLevel: types.Logger_info, + DefaultLevel: types.Logger_info, + }, + }, + expectedSetValue: loggerv1.SetLogLevelRequest_TRACE, + expectReturnCode: 1, + expectStderr: `Error: the value never is not a valid setting +`, + }, + { + name: "bizzarro world, returns neither logger nor error", + args: []string{"-level", "info", "-output", "pretty"}, + server: &mockLoggerServer{ + returnLogger: nil, + }, + expectReturnCode: 1, + expectStderr: `Error: error fetching logger: rpc error: code = Internal desc = grpc: error while marshaling: proto: Marshal called with nil +`, + }, + { + name: "No attribute set, cli returns error", + args: []string{"-output", "pretty"}, + server: &mockLoggerServer{ + returnLogger: &types.Logger{ + CurrentLevel: types.Logger_info, + DefaultLevel: types.Logger_info, + }, + }, + expectReturnCode: 1, + expectStderr: `Error: a value (-level) must be set +`, + }, + { + name: "bizzarro world, set to trace, logger unadjusted from (info,info)", + args: []string{"-level", "trace", "-output", "pretty"}, + server: &mockLoggerServer{ + returnLogger: &types.Logger{ + CurrentLevel: types.Logger_info, + DefaultLevel: types.Logger_info, + }, + }, + expectedSetValue: loggerv1.SetLogLevelRequest_TRACE, + expectReturnCode: 0, + expectStdout: `Logger Level : info +Logger Default: info + +`, + }, + } { + t.Run(tt.name, func(t *testing.T) { + test := setupCliTest(t, tt.server, logger.NewSetCommandWithEnv) + returnCode := test.client.Run(append(test.args, tt.args...)) + require.Equal(t, tt.expectStdout, test.stdout.String()) + require.Equal(t, tt.expectStderr, test.stderr.String()) + require.Equal(t, tt.expectReturnCode, returnCode) + }) + } +} diff --git a/pkg/server/api/logger/v1/service.go b/pkg/server/api/logger/v1/service.go index 4d37c45f21..0adb28f223 100644 --- a/pkg/server/api/logger/v1/service.go +++ b/pkg/server/api/logger/v1/service.go @@ -44,13 +44,13 @@ func (service *Service) GetLogger(ctx context.Context, req *loggerv1.GetLoggerRe func (service *Service) SetLogLevel(ctx context.Context, req *loggerv1.SetLogLevelRequest) (*types.Logger, error) { logrus.WithFields(logrus.Fields{ - "RequestLevel": loggerv1.SetLogLevelRequest_SetValue_name[int32(req.LogLevel)], + "RequestLevel": loggerv1.SetLogLevelRequest_SetValue_name[int32(req.SetLevel)], }).Info("SetLogger Called") - setLevel := loggerv1.SetLogLevelRequest_SetValue(req.LogLevel) + setLevel := loggerv1.SetLogLevelRequest_SetValue(req.SetLevel) if setLevel == loggerv1.SetLogLevelRequest_DEFAULT { logrus.SetLevel(service.DefaultLevel) } else { - logrus.SetLevel(logrus.Level(req.LogLevel)) + logrus.SetLevel(logrus.Level(req.SetLevel)) } logger := &types.Logger{ CurrentLevel: types.Logger_LogLevel(logrus.GetLevel()), From e2749625caf3316f74681f3cde18148a77c6b265 Mon Sep 17 00:00:00 2001 From: Edwin Buck Date: Fri, 1 Mar 2024 09:36:44 -0600 Subject: [PATCH 06/20] Rework server commands to use new API Signed-off-by: Edwin Buck --- cmd/spire-server/cli/logger/get_test.go | 56 +++++++------- cmd/spire-server/cli/logger/mocks_test.go | 12 ++- cmd/spire-server/cli/logger/printers.go | 29 ++++++- cmd/spire-server/cli/logger/printers_test.go | 12 +-- cmd/spire-server/cli/logger/set.go | 41 ++++++---- cmd/spire-server/cli/logger/set_test.go | 81 ++++++++------------ cmd/spire-server/cli/run/run.go | 2 +- go.mod | 2 +- go.sum | 4 +- pkg/server/api/logger/v1/levels.go | 27 +++++++ pkg/server/api/logger/v1/service.go | 50 +++++++----- pkg/server/config.go | 2 +- pkg/server/endpoints/config.go | 4 +- pkg/server/endpoints/endpoints.go | 4 +- pkg/server/server.go | 4 +- 15 files changed, 195 insertions(+), 135 deletions(-) create mode 100644 pkg/server/api/logger/v1/levels.go diff --git a/cmd/spire-server/cli/logger/get_test.go b/cmd/spire-server/cli/logger/get_test.go index e562ae78a8..13794fe6f5 100644 --- a/cmd/spire-server/cli/logger/get_test.go +++ b/cmd/spire-server/cli/logger/get_test.go @@ -47,13 +47,13 @@ func TestGet(t *testing.T) { args: []string{"-output", "pretty"}, server: &mockLoggerServer{ returnLogger: &types.Logger{ - CurrentLevel: types.Logger_info, - DefaultLevel: types.Logger_info, + CurrentLevel: types.LogLevel_INFO, + LaunchLevel: types.LogLevel_INFO, }, }, expectReturnCode: 0, - expectStdout: `Logger Level : info -Logger Default: info + expectStdout: `Logger Level : info +Launch Level : info `, }, @@ -62,13 +62,13 @@ Logger Default: info args: []string{"-output", "pretty"}, server: &mockLoggerServer{ returnLogger: &types.Logger{ - CurrentLevel: types.Logger_warn, - DefaultLevel: types.Logger_debug, + CurrentLevel: types.LogLevel_WARN, + LaunchLevel: types.LogLevel_DEBUG, }, }, expectReturnCode: 0, - expectStdout: `Logger Level : warning -Logger Default: debug + expectStdout: `Logger Level : warning +Launch Level : debug `, }, @@ -77,13 +77,13 @@ Logger Default: debug args: []string{"-output", "pretty"}, server: &mockLoggerServer{ returnLogger: &types.Logger{ - CurrentLevel: types.Logger_trace, - DefaultLevel: types.Logger_error, + CurrentLevel: types.LogLevel_TRACE, + LaunchLevel: types.LogLevel_ERROR, }, }, expectReturnCode: 0, - expectStdout: `Logger Level : trace -Logger Default: error + expectStdout: `Logger Level : trace +Launch Level : error `, }, @@ -92,13 +92,13 @@ Logger Default: error args: []string{"-output", "pretty"}, server: &mockLoggerServer{ returnLogger: &types.Logger{ - CurrentLevel: types.Logger_fatal, - DefaultLevel: types.Logger_panic, + CurrentLevel: types.LogLevel_FATAL, + LaunchLevel: types.LogLevel_PANIC, }, }, expectReturnCode: 0, - expectStdout: `Logger Level : fatal -Logger Default: panic + expectStdout: `Logger Level : fatal +Launch Level : panic `, }, @@ -107,12 +107,12 @@ Logger Default: panic args: []string{"-output", "json"}, server: &mockLoggerServer{ returnLogger: &types.Logger{ - CurrentLevel: types.Logger_info, - DefaultLevel: types.Logger_info, + CurrentLevel: types.LogLevel_INFO, + LaunchLevel: types.LogLevel_INFO, }, }, expectReturnCode: 0, - expectStdout: `{"current_level":"info","default_level":"info"} + expectStdout: `{"current_level":"INFO","launch_level":"INFO"} `, }, { @@ -120,12 +120,12 @@ Logger Default: panic args: []string{"-output", "json"}, server: &mockLoggerServer{ returnLogger: &types.Logger{ - CurrentLevel: types.Logger_warn, - DefaultLevel: types.Logger_debug, + CurrentLevel: types.LogLevel_WARN, + LaunchLevel: types.LogLevel_DEBUG, }, }, expectReturnCode: 0, - expectStdout: `{"current_level":"warn","default_level":"debug"} + expectStdout: `{"current_level":"WARN","launch_level":"DEBUG"} `, }, { @@ -133,12 +133,12 @@ Logger Default: panic args: []string{"-output", "json"}, server: &mockLoggerServer{ returnLogger: &types.Logger{ - CurrentLevel: types.Logger_trace, - DefaultLevel: types.Logger_error, + CurrentLevel: types.LogLevel_TRACE, + LaunchLevel: types.LogLevel_ERROR, }, }, expectReturnCode: 0, - expectStdout: `{"current_level":"trace","default_level":"error"} + expectStdout: `{"current_level":"TRACE","launch_level":"ERROR"} `, }, { @@ -146,12 +146,12 @@ Logger Default: panic args: []string{"-output", "json"}, server: &mockLoggerServer{ returnLogger: &types.Logger{ - CurrentLevel: types.Logger_fatal, - DefaultLevel: types.Logger_panic, + CurrentLevel: types.LogLevel_FATAL, + LaunchLevel: types.LogLevel_PANIC, }, }, expectReturnCode: 0, - expectStdout: `{"current_level":"fatal","default_level":"panic"} + expectStdout: `{"current_level":"FATAL","launch_level":"PANIC"} `, }, { diff --git a/cmd/spire-server/cli/logger/mocks_test.go b/cmd/spire-server/cli/logger/mocks_test.go index ff67c9ba4e..a4a120c4c1 100644 --- a/cmd/spire-server/cli/logger/mocks_test.go +++ b/cmd/spire-server/cli/logger/mocks_test.go @@ -12,8 +12,8 @@ import ( "github.com/spiffe/spire/cmd/spire-server/cli/common" commoncli "github.com/spiffe/spire/pkg/common/cli" "google.golang.org/grpc" - "github.com/spiffe/spire-api-sdk/proto/spire/api/types" loggerv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/logger/v1" + "github.com/spiffe/spire-api-sdk/proto/spire/api/types" ) // an input/output capture struct @@ -70,7 +70,7 @@ func setupCliTest(t *testing.T, server *mockLoggerServer, newClient func(*common type mockLoggerServer struct { loggerv1.UnimplementedLoggerServer - receivedSetValue loggerv1.SetLogLevelRequest_SetValue + receivedSetValue *types.LogLevel returnLogger *types.Logger returnErr error } @@ -81,10 +81,16 @@ func (s *mockLoggerServer) GetLogger(_ context.Context, _ *loggerv1.GetLoggerReq } func (s *mockLoggerServer) SetLogLevel(_ context.Context, req *loggerv1.SetLogLevelRequest) (*types.Logger, error) { - s.receivedSetValue = req.SetLevel + s.receivedSetValue = &req.NewLevel + return s.returnLogger, s.returnErr +} + +func (s *mockLoggerServer) ResetLogLevel(_ context.Context, req *loggerv1.ResetLogLevelRequest) (*types.Logger, error) { + s.receivedSetValue = nil return s.returnLogger, s.returnErr } + var _ io.Writer = &errorWriter{} type errorWriter struct { diff --git a/cmd/spire-server/cli/logger/printers.go b/cmd/spire-server/cli/logger/printers.go index 73b5c4c8a7..70d9dddf01 100644 --- a/cmd/spire-server/cli/logger/printers.go +++ b/cmd/spire-server/cli/logger/printers.go @@ -2,17 +2,38 @@ package logger import ( "errors" - "github.com/sirupsen/logrus" - "github.com/spiffe/spire-api-sdk/proto/spire/api/types" + "fmt" + + apitype "github.com/spiffe/spire-api-sdk/proto/spire/api/types" commoncli "github.com/spiffe/spire/pkg/common/cli" + serverlogger "github.com/spiffe/spire/pkg/server/api/logger/v1" ) func PrettyPrintLogger(env *commoncli.Env, results ...any) error { - logger, ok := results[0].(*types.Logger) + apiLogger, ok := results[0].(*apitype.Logger) if !ok { return errors.New("internal error: logger not found; please report this as a bug") } - if err := env.Printf("Logger Level : %s\nLogger Default: %s\n\n", logrus.Level(logger.CurrentLevel), logrus.Level(logger.DefaultLevel)); err != nil { + + logrusCurrent, found := serverlogger.LogrusLevel[apiLogger.CurrentLevel] + if !found { + return errors.New("internal error: returned current log level is undefined; please report this as a bug.") + } + currentText, err := logrusCurrent.MarshalText() + if err != nil { + return fmt.Errorf("internal error: logrus log level %d has no name; please report this as a bug.", logrusCurrent) + } + + logrusLaunch, found := serverlogger.LogrusLevel[apiLogger.LaunchLevel] + if !found { + return errors.New("internal error: returned current log level is undefined; please report this as a bug.") + } + launchText, err := logrusLaunch.MarshalText() + if err != nil { + return fmt.Errorf("internal error: logrus log level %d has no name; please report this as a bug.", logrusLaunch) + } + + if err := env.Printf("Logger Level : %s\nLaunch Level : %s\n\n", currentText, launchText); err != nil { return err } return nil diff --git a/cmd/spire-server/cli/logger/printers_test.go b/cmd/spire-server/cli/logger/printers_test.go index 8222141dc8..32b760dc68 100644 --- a/cmd/spire-server/cli/logger/printers_test.go +++ b/cmd/spire-server/cli/logger/printers_test.go @@ -24,11 +24,11 @@ func TestPrettyPrintLogger(t *testing.T) { { name: "test", logger: &types.Logger{ - CurrentLevel: types.Logger_debug, - DefaultLevel: types.Logger_info, + CurrentLevel: types.LogLevel_DEBUG, + LaunchLevel: types.LogLevel_INFO, }, - expectedStdout: `Logger Level : debug -Logger Default: info + expectedStdout: `Logger Level : debug +Launch Level : info `, }, @@ -38,8 +38,8 @@ Logger Default: info ReturnError: errors.New("cannot write"), }, logger: &types.Logger{ - CurrentLevel: types.Logger_debug, - DefaultLevel: types.Logger_info, + CurrentLevel: types.LogLevel_DEBUG, + LaunchLevel: types.LogLevel_INFO, }, expectedError: errors.New("cannot write"), }, diff --git a/cmd/spire-server/cli/logger/set.go b/cmd/spire-server/cli/logger/set.go index 63ec16448a..23da518028 100644 --- a/cmd/spire-server/cli/logger/set.go +++ b/cmd/spire-server/cli/logger/set.go @@ -8,7 +8,10 @@ import ( "github.com/mitchellh/cli" api "github.com/spiffe/spire-api-sdk/proto/spire/api/server/logger/v1" + apitype "github.com/spiffe/spire-api-sdk/proto/spire/api/types" + "github.com/sirupsen/logrus" "github.com/spiffe/spire/cmd/spire-server/util" + serverlogger "github.com/spiffe/spire/pkg/server/api/logger/v1" commoncli "github.com/spiffe/spire/pkg/common/cli" "github.com/spiffe/spire/pkg/common/cliprinter" ) @@ -42,32 +45,38 @@ func (_ *setCommand) Synopsis() string { // Adds additional flags specific to the command. func (c *setCommand) AppendFlags(fs *flag.FlagSet) { - fs.StringVar(&c.newLevel, "level", "", "The new log level, one of (panic, fatal, error, warn, info, debug, trace, default)") + fs.StringVar(&c.newLevel, "level", "", "The new log level, one of (panic, fatal, error, warn, info, debug, trace, launch)") cliprinter.AppendFlagWithCustomPretty(&c.printer, fs, c.env, c.prettyPrintLogger) } // The routine that executes the command func (c *setCommand) Run(ctx context.Context, _ *commoncli.Env, serverClient util.ServerClient) error { - if c.newLevel != "" { - fmt.Errorf("the newLevel is %s", c.newLevel) - grpc_key := strings.ToUpper(c.newLevel) - value, found := api.SetLogLevelRequest_SetValue_value[grpc_key] - if !found { + if c.newLevel == "" { + return fmt.Errorf("a value (-level) must be set") + } + level := strings.ToLower(c.newLevel) + var logger *apitype.Logger + var err error + if level == "launch" { + logger, err = serverClient.NewLoggerClient().ResetLogLevel(ctx, &api.ResetLogLevelRequest{}) + } else { + logrusLevel, err := logrus.ParseLevel(level) + if err != nil { return fmt.Errorf("the value %s is not a valid setting", c.newLevel) } - - logger, err := serverClient.NewLoggerClient().SetLogLevel(ctx, &api.SetLogLevelRequest{ - SetLevel: api.SetLogLevelRequest_SetValue(value), - }) - if err != nil { - return fmt.Errorf("error fetching logger: %w", err) + apiLevel, found := serverlogger.ApiLevel[logrusLevel] + if !found { + return fmt.Errorf("the logrus level %d could not be transformed into an api log level", logrusLevel) } - - return c.printer.PrintProto(logger) + logger, err = serverClient.NewLoggerClient().SetLogLevel(ctx, &api.SetLogLevelRequest{ + NewLevel: apiLevel, + }) } - - return fmt.Errorf("a value (-level) must be set") + if err != nil { + return fmt.Errorf("error fetching logger: %w", err) + } + return c.printer.PrintProto(logger) } func (l* setCommand) prettyPrintLogger(env *commoncli.Env, results ...any) error { diff --git a/cmd/spire-server/cli/logger/set_test.go b/cmd/spire-server/cli/logger/set_test.go index 4a6227961b..5ced0bc164 100644 --- a/cmd/spire-server/cli/logger/set_test.go +++ b/cmd/spire-server/cli/logger/set_test.go @@ -5,14 +5,13 @@ import ( "github.com/stretchr/testify/require" "github.com/spiffe/spire/cmd/spire-server/cli/logger" - loggerv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/logger/v1" "github.com/spiffe/spire-api-sdk/proto/spire/api/types" ) var ( setUsage = `Usage of logger set: -level string - The new log level, one of (panic, fatal, error, warn, info, debug, trace, default) + The new log level, one of (panic, fatal, error, warn, info, debug, trace, launch) -output value Desired output format (pretty, json); default: pretty. -socketPath string @@ -40,7 +39,7 @@ func TestSet(t *testing.T) { // input args []string // expected items - expectedSetValue loggerv1.SetLogLevelRequest_SetValue + expectedSetValue types.LogLevel expectReturnCode int expectStdout string expectStderr string @@ -50,14 +49,14 @@ func TestSet(t *testing.T) { args: []string{"-level", "debug", "-output", "pretty"}, server: &mockLoggerServer{ returnLogger: &types.Logger{ - CurrentLevel: types.Logger_debug, - DefaultLevel: types.Logger_info, + CurrentLevel: types.LogLevel_DEBUG, + LaunchLevel: types.LogLevel_INFO, }, }, - expectedSetValue: loggerv1.SetLogLevelRequest_DEBUG, + expectedSetValue: types.LogLevel_DEBUG, expectReturnCode: 0, - expectStdout: `Logger Level : debug -Logger Default: info + expectStdout: `Logger Level : debug +Launch Level : info `, }, @@ -66,30 +65,29 @@ Logger Default: info args: []string{"-level", "warn", "-output", "pretty"}, server: &mockLoggerServer{ returnLogger: &types.Logger{ - CurrentLevel: types.Logger_warn, - DefaultLevel: types.Logger_debug, + CurrentLevel: types.LogLevel_WARN, + LaunchLevel: types.LogLevel_DEBUG, }, }, - expectedSetValue: loggerv1.SetLogLevelRequest_WARN, + expectedSetValue: types.LogLevel_WARN, expectReturnCode: 0, - expectStdout: `Logger Level : warning -Logger Default: debug + expectStdout: `Logger Level : warning +Launch Level : debug `, }, { - name: "set to default, configured to error, using pretty output", - args: []string{"-level", "default", "-output", "pretty"}, + name: "set to launch, configured to error, using pretty output", + args: []string{"-level", "launch", "-output", "pretty"}, server: &mockLoggerServer{ returnLogger: &types.Logger{ - CurrentLevel: types.Logger_error, - DefaultLevel: types.Logger_error, + CurrentLevel: types.LogLevel_ERROR, + LaunchLevel: types.LogLevel_ERROR, }, }, - expectedSetValue: loggerv1.SetLogLevelRequest_DEFAULT, expectReturnCode: 0, - expectStdout: `Logger Level : error -Logger Default: error + expectStdout: `Logger Level : error +Launch Level : error `, }, @@ -98,14 +96,14 @@ Logger Default: error args: []string{"-level", "panic", "-output", "pretty"}, server: &mockLoggerServer{ returnLogger: &types.Logger{ - CurrentLevel: types.Logger_panic, - DefaultLevel: types.Logger_fatal, + CurrentLevel: types.LogLevel_PANIC, + LaunchLevel: types.LogLevel_FATAL, }, }, - expectedSetValue: loggerv1.SetLogLevelRequest_PANIC, + expectedSetValue: types.LogLevel_PANIC, expectReturnCode: 0, - expectStdout: `Logger Level : panic -Logger Default: fatal + expectStdout: `Logger Level : panic +Launch Level : fatal `, }, @@ -114,23 +112,12 @@ Logger Default: fatal args: []string{"-level", "never", "-output", "pretty"}, server: &mockLoggerServer{ returnLogger: &types.Logger{ - CurrentLevel: types.Logger_info, - DefaultLevel: types.Logger_info, + CurrentLevel: types.LogLevel_INFO, + LaunchLevel: types.LogLevel_INFO, }, }, - expectedSetValue: loggerv1.SetLogLevelRequest_TRACE, expectReturnCode: 1, expectStderr: `Error: the value never is not a valid setting -`, - }, - { - name: "bizzarro world, returns neither logger nor error", - args: []string{"-level", "info", "-output", "pretty"}, - server: &mockLoggerServer{ - returnLogger: nil, - }, - expectReturnCode: 1, - expectStderr: `Error: error fetching logger: rpc error: code = Internal desc = grpc: error while marshaling: proto: Marshal called with nil `, }, { @@ -138,8 +125,8 @@ Logger Default: fatal args: []string{"-output", "pretty"}, server: &mockLoggerServer{ returnLogger: &types.Logger{ - CurrentLevel: types.Logger_info, - DefaultLevel: types.Logger_info, + CurrentLevel: types.LogLevel_INFO, + LaunchLevel: types.LogLevel_INFO, }, }, expectReturnCode: 1, @@ -151,14 +138,14 @@ Logger Default: fatal args: []string{"-level", "trace", "-output", "pretty"}, server: &mockLoggerServer{ returnLogger: &types.Logger{ - CurrentLevel: types.Logger_info, - DefaultLevel: types.Logger_info, + CurrentLevel: types.LogLevel_INFO, + LaunchLevel: types.LogLevel_INFO, }, }, - expectedSetValue: loggerv1.SetLogLevelRequest_TRACE, + expectedSetValue: types.LogLevel_TRACE, expectReturnCode: 0, - expectStdout: `Logger Level : info -Logger Default: info + expectStdout: `Logger Level : info +Launch Level : info `, }, @@ -166,9 +153,9 @@ Logger Default: info t.Run(tt.name, func(t *testing.T) { test := setupCliTest(t, tt.server, logger.NewSetCommandWithEnv) returnCode := test.client.Run(append(test.args, tt.args...)) - require.Equal(t, tt.expectStdout, test.stdout.String()) - require.Equal(t, tt.expectStderr, test.stderr.String()) require.Equal(t, tt.expectReturnCode, returnCode) + require.Equal(t, tt.expectStderr, test.stderr.String()) + require.Equal(t, tt.expectStdout, test.stdout.String()) }) } } diff --git a/cmd/spire-server/cli/run/run.go b/cmd/spire-server/cli/run/run.go index 5fc39c274f..0dbab00327 100644 --- a/cmd/spire-server/cli/run/run.go +++ b/cmd/spire-server/cli/run/run.go @@ -385,7 +385,7 @@ func NewServerConfig(c *Config, logOptions []log.Option, allowUnknownConfig bool } logger, err := log.NewLogger(logOptions...) - sc.DefaultLogLevel, _ = logrus.ParseLevel(c.Server.LogLevel) + sc.LaunchLogLevel, _ = logrus.ParseLevel(c.Server.LogLevel) if err != nil { return nil, fmt.Errorf("could not start logger: %w", err) } diff --git a/go.mod b/go.mod index d99405dc7c..5f77fb8128 100644 --- a/go.mod +++ b/go.mod @@ -69,7 +69,7 @@ require ( github.com/sigstore/sigstore v1.8.2 github.com/sirupsen/logrus v1.9.3 github.com/spiffe/go-spiffe/v2 v2.1.7 - github.com/spiffe/spire-api-sdk v1.2.5-0.20231107161112-ba57e0e943a2 + github.com/spiffe/spire-api-sdk v1.2.5-0.20240222231036-08f5a1ab98c6 github.com/spiffe/spire-plugin-sdk v1.4.4-0.20230721151831-bf67dde4721d github.com/stretchr/testify v1.9.0 github.com/uber-go/tally/v4 v4.1.12 diff --git a/go.sum b/go.sum index ae299834c8..31ab9cdab5 100644 --- a/go.sum +++ b/go.sum @@ -1393,8 +1393,8 @@ github.com/spf13/viper v1.18.2/go.mod h1:EKmWIqdnk5lOcmR72yw6hS+8OPYcwD0jteitLMV github.com/spiffe/go-spiffe/v2 v2.1.6/go.mod h1:eVDqm9xFvyqao6C+eQensb9ZPkyNEeaUbqbBpOhBnNk= github.com/spiffe/go-spiffe/v2 v2.1.7 h1:VUkM1yIyg/x8X7u1uXqSRVRCdMdfRIEdFBzpqoeASGk= github.com/spiffe/go-spiffe/v2 v2.1.7/go.mod h1:QJDGdhXllxjxvd5B+2XnhhXB/+rC8gr+lNrtOryiWeE= -github.com/spiffe/spire-api-sdk v1.2.5-0.20231107161112-ba57e0e943a2 h1:EKSBig+9oEvyLUi80aE/88UHjoNCqlNGTFTjm02F+fk= -github.com/spiffe/spire-api-sdk v1.2.5-0.20231107161112-ba57e0e943a2/go.mod h1:4uuhFlN6KBWjACRP3xXwrOTNnvaLp1zJs8Lribtr4fI= +github.com/spiffe/spire-api-sdk v1.2.5-0.20240222231036-08f5a1ab98c6 h1:gCctMhffEF4KcrLP85qQwOeQoHCMMYlDL1HR0fEZ+sE= +github.com/spiffe/spire-api-sdk v1.2.5-0.20240222231036-08f5a1ab98c6/go.mod h1:4uuhFlN6KBWjACRP3xXwrOTNnvaLp1zJs8Lribtr4fI= github.com/spiffe/spire-plugin-sdk v1.4.4-0.20230721151831-bf67dde4721d h1:LCRQGU6vOqKLfRrG+GJQrwMwDILcAddAEIf4/1PaSVc= github.com/spiffe/spire-plugin-sdk v1.4.4-0.20230721151831-bf67dde4721d/go.mod h1:GA6o2PVLwyJdevT6KKt5ZXCY/ziAPna13y/seGk49Ik= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= diff --git a/pkg/server/api/logger/v1/levels.go b/pkg/server/api/logger/v1/levels.go new file mode 100644 index 0000000000..bcf99339ba --- /dev/null +++ b/pkg/server/api/logger/v1/levels.go @@ -0,0 +1,27 @@ +package logger + +import ( + logrus "github.com/sirupsen/logrus" + apitype "github.com/spiffe/spire-api-sdk/proto/spire/api/types" +) + +var ApiLevel = map[logrus.Level]apitype.LogLevel{ + logrus.PanicLevel: apitype.LogLevel_PANIC, + logrus.FatalLevel: apitype.LogLevel_FATAL, + logrus.ErrorLevel: apitype.LogLevel_ERROR, + logrus.WarnLevel: apitype.LogLevel_WARN, + logrus.InfoLevel: apitype.LogLevel_INFO, + logrus.DebugLevel: apitype.LogLevel_DEBUG, + logrus.TraceLevel: apitype.LogLevel_TRACE, +} + +var LogrusLevel = map[apitype.LogLevel]logrus.Level{ + apitype.LogLevel_PANIC: logrus.PanicLevel, + apitype.LogLevel_FATAL: logrus.FatalLevel, + apitype.LogLevel_ERROR: logrus.ErrorLevel, + apitype.LogLevel_WARN: logrus.WarnLevel, + apitype.LogLevel_INFO: logrus.InfoLevel, + apitype.LogLevel_DEBUG: logrus.DebugLevel, + apitype.LogLevel_TRACE: logrus.TraceLevel, +} + diff --git a/pkg/server/api/logger/v1/service.go b/pkg/server/api/logger/v1/service.go index 0adb28f223..dab1003642 100644 --- a/pkg/server/api/logger/v1/service.go +++ b/pkg/server/api/logger/v1/service.go @@ -1,31 +1,33 @@ package logger import ( + "fmt" "context" + "strings" loggerv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/logger/v1" - "github.com/spiffe/spire-api-sdk/proto/spire/api/types" + apitype "github.com/spiffe/spire-api-sdk/proto/spire/api/types" "github.com/sirupsen/logrus" "google.golang.org/grpc" ) type Config struct { - DefaultLevel logrus.Level + LaunchLevel logrus.Level } type Service struct { loggerv1.UnsafeLoggerServer - DefaultLevel logrus.Level + LaunchLevel logrus.Level } func New(config Config) *Service { logrus.WithFields(logrus.Fields{ - "DefaultLevel": config.DefaultLevel, + "LaunchLevel": config.LaunchLevel, }).Info("logger service Configured") return &Service{ - DefaultLevel: config.DefaultLevel, + LaunchLevel: config.LaunchLevel, } } @@ -33,28 +35,36 @@ func RegisterService(server *grpc.Server, service *Service) { loggerv1.RegisterLoggerServer(server, service) } -func (service *Service) GetLogger(ctx context.Context, req *loggerv1.GetLoggerRequest) (*types.Logger, error) { +func (service *Service) GetLogger(ctx context.Context, req *loggerv1.GetLoggerRequest) (*apitype.Logger, error) { logrus.Info("GetLogger Called") - logger := &types.Logger{ - CurrentLevel: types.Logger_LogLevel(logrus.GetLevel()), - DefaultLevel: types.Logger_LogLevel(service.DefaultLevel), + logger := &apitype.Logger{ + CurrentLevel: ApiLevel[logrus.GetLevel()], + LaunchLevel: ApiLevel[service.LaunchLevel], } return logger, nil } -func (service *Service) SetLogLevel(ctx context.Context, req *loggerv1.SetLogLevelRequest) (*types.Logger, error) { +func (service *Service) SetLogLevel(ctx context.Context, req *loggerv1.SetLogLevelRequest) (*apitype.Logger, error) { + if req.NewLevel == apitype.LogLevel_UNSPECIFIED { + return nil, fmt.Errorf("Invalid request NewLevel value cannot be LogLevel_UNSPECIFIED") + } logrus.WithFields(logrus.Fields{ - "RequestLevel": loggerv1.SetLogLevelRequest_SetValue_name[int32(req.SetLevel)], - }).Info("SetLogger Called") - setLevel := loggerv1.SetLogLevelRequest_SetValue(req.SetLevel) - if setLevel == loggerv1.SetLogLevelRequest_DEFAULT { - logrus.SetLevel(service.DefaultLevel) - } else { - logrus.SetLevel(logrus.Level(req.SetLevel)) + "RequestLevel": strings.ToLower(apitype.LogLevel_name[int32(req.NewLevel)]), + }).Info("SetLogLevel Called") + logrus.SetLevel(logrus.Level(req.NewLevel)) + logger := &apitype.Logger{ + CurrentLevel: ApiLevel[logrus.GetLevel()], + LaunchLevel: ApiLevel[service.LaunchLevel], } - logger := &types.Logger{ - CurrentLevel: types.Logger_LogLevel(logrus.GetLevel()), - DefaultLevel: types.Logger_LogLevel(service.DefaultLevel), + return logger, nil +} + +func (service *Service) ResetLogLevel(ctx context.Context, req *loggerv1.ResetLogLevelRequest) (*apitype.Logger, error) { + logrus.Info("ResetLogLevel Called") + logrus.SetLevel(service.LaunchLevel) + logger := &apitype.Logger{ + CurrentLevel: ApiLevel[logrus.GetLevel()], + LaunchLevel: ApiLevel[service.LaunchLevel], } return logger, nil } diff --git a/pkg/server/config.go b/pkg/server/config.go index 776cdf2c8a..cf9eec8687 100644 --- a/pkg/server/config.go +++ b/pkg/server/config.go @@ -24,7 +24,7 @@ type Config struct { Log logrus.FieldLogger - DefaultLogLevel logrus.Level + LaunchLogLevel logrus.Level // LogReopener facilitates handling a signal to rotate log file. LogReopener func(context.Context) error diff --git a/pkg/server/endpoints/config.go b/pkg/server/endpoints/config.go index 47f1df3387..af6c4cd3fa 100644 --- a/pkg/server/endpoints/config.go +++ b/pkg/server/endpoints/config.go @@ -66,7 +66,7 @@ type Config struct { Log logrus.FieldLogger // The default (original config) log level - DefaultLogLevel logrus.Level + LaunchLogLevel logrus.Level Metrics telemetry.Metrics @@ -164,7 +164,7 @@ func (c *Config) makeAPIServers(entryFetcher api.AuthorizedEntryFetcher) APIServ DataStore: ds, }), LoggerServer: loggerv1.New(loggerv1.Config{ - DefaultLevel: c.DefaultLogLevel, + LaunchLevel: c.LaunchLogLevel, }), SVIDServer: svidv1.New(svidv1.Config{ TrustDomain: c.TrustDomain, diff --git a/pkg/server/endpoints/endpoints.go b/pkg/server/endpoints/endpoints.go index 6bb1e5aa87..1d767999e5 100644 --- a/pkg/server/endpoints/endpoints.go +++ b/pkg/server/endpoints/endpoints.go @@ -177,7 +177,7 @@ func (e *Endpoints) ListenAndServe(ctx context.Context) error { tcpServer := e.createTCPServer(ctx, unaryInterceptor, streamInterceptor) udsServer := e.createUDSServer(unaryInterceptor, streamInterceptor) - // TCP and UDP + // TCP and UDS agentv1.RegisterAgentServer(tcpServer, e.APIServers.AgentServer) agentv1.RegisterAgentServer(udsServer, e.APIServers.AgentServer) bundlev1.RegisterBundleServer(tcpServer, e.APIServers.BundleServer) @@ -189,7 +189,7 @@ func (e *Endpoints) ListenAndServe(ctx context.Context) error { trustdomainv1.RegisterTrustDomainServer(tcpServer, e.APIServers.TrustDomainServer) trustdomainv1.RegisterTrustDomainServer(udsServer, e.APIServers.TrustDomainServer) - // UDP only + // UDS only loggerv1.RegisterLoggerServer(udsServer, e.APIServers.LoggerServer) grpc_health_v1.RegisterHealthServer(udsServer, e.APIServers.HealthServer) debugv1_pb.RegisterDebugServer(udsServer, e.APIServers.DebugServer) diff --git a/pkg/server/server.go b/pkg/server/server.go index d2d2b86907..4eaa4cab9d 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -76,7 +76,7 @@ func (s *Server) run(ctx context.Context) (err error) { }).Info("Configured") s.config.Log.WithFields(logrus.Fields{ - "DefaultLogLevel": s.config.DefaultLogLevel, + "LaunchLogLevel": s.config.LaunchLogLevel, }).Info("Log Level") // create the data directory if needed @@ -390,7 +390,7 @@ func (s *Server) newEndpointsServer(ctx context.Context, catalog catalog.Catalog Catalog: catalog, ServerCA: serverCA, Log: s.config.Log.WithField(telemetry.SubsystemName, telemetry.Endpoints), - DefaultLogLevel: s.config.DefaultLogLevel, + LaunchLogLevel: s.config.LaunchLogLevel, Metrics: metrics, JWTKeyPublisher: jwtKeyPublisher, RateLimit: s.config.RateLimit, From 25ef54093d4b684cfcc604f5595a3f5c3d6b8f17 Mon Sep 17 00:00:00 2001 From: Edwin Buck Date: Sun, 3 Mar 2024 09:50:57 -0600 Subject: [PATCH 07/20] Fix off-by one log level setting, introduced with new API change. Signed-off-by: Edwin Buck --- pkg/server/api/logger/v1/service.go | 5 +++-- pkg/server/authpolicy/policy_data.json | 5 +++++ pkg/server/endpoints/middleware.go | 1 + 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/pkg/server/api/logger/v1/service.go b/pkg/server/api/logger/v1/service.go index dab1003642..a8cb3e8338 100644 --- a/pkg/server/api/logger/v1/service.go +++ b/pkg/server/api/logger/v1/service.go @@ -49,9 +49,10 @@ func (service *Service) SetLogLevel(ctx context.Context, req *loggerv1.SetLogLev return nil, fmt.Errorf("Invalid request NewLevel value cannot be LogLevel_UNSPECIFIED") } logrus.WithFields(logrus.Fields{ - "RequestLevel": strings.ToLower(apitype.LogLevel_name[int32(req.NewLevel)]), + "ApiLogLevel": strings.ToLower(apitype.LogLevel_name[int32(req.NewLevel)]), + "LogrusLevel": LogrusLevel[req.NewLevel].String(), }).Info("SetLogLevel Called") - logrus.SetLevel(logrus.Level(req.NewLevel)) + logrus.SetLevel(LogrusLevel[req.NewLevel]) logger := &apitype.Logger{ CurrentLevel: ApiLevel[logrus.GetLevel()], LaunchLevel: ApiLevel[service.LaunchLevel], diff --git a/pkg/server/authpolicy/policy_data.json b/pkg/server/authpolicy/policy_data.json index d79b7ddb88..e14d134c72 100644 --- a/pkg/server/authpolicy/policy_data.json +++ b/pkg/server/authpolicy/policy_data.json @@ -123,6 +123,11 @@ "allow_admin": true, "allow_local": true }, + { + "full_method": "/spire.api.server.logger.v1.Logger/ResetLogLevel", + "allow_admin": true, + "allow_local": true + }, { "full_method": "/spire.api.server.agent.v1.Agent/CountAgents", "allow_admin": true, diff --git a/pkg/server/endpoints/middleware.go b/pkg/server/endpoints/middleware.go index a54dd2b846..c8df56389d 100644 --- a/pkg/server/endpoints/middleware.go +++ b/pkg/server/endpoints/middleware.go @@ -155,6 +155,7 @@ func RateLimits(config RateLimitConfig) map[string]api.RateLimiter { "/spire.api.server.entry.v1.Entry/SyncAuthorizedEntries": noLimit, "/spire.api.server.logger.v1.Logger/GetLogger": noLimit, "/spire.api.server.logger.v1.Logger/SetLogLevel": noLimit, + "/spire.api.server.logger.v1.Logger/ResetLogLevel": noLimit, "/spire.api.server.agent.v1.Agent/CountAgents": noLimit, "/spire.api.server.agent.v1.Agent/ListAgents": noLimit, "/spire.api.server.agent.v1.Agent/GetAgent": noLimit, From 801e8af39c3acdd5b735810540509ba481394f57 Mon Sep 17 00:00:00 2001 From: Edwin Buck Date: Mon, 4 Mar 2024 15:12:22 -0600 Subject: [PATCH 08/20] Fix the issue of not configuring the log.Logger (from pkg) or logrus.Logger. Signed-off-by: Edwin Buck --- cmd/spire-server/cli/run/run.go | 1 + pkg/server/api/logger/v1/service.go | 11 ++++++++--- pkg/server/config.go | 3 +++ pkg/server/endpoints/config.go | 7 ++++++- pkg/server/server.go | 1 + 5 files changed, 19 insertions(+), 4 deletions(-) diff --git a/cmd/spire-server/cli/run/run.go b/cmd/spire-server/cli/run/run.go index 0dbab00327..acd72e36a2 100644 --- a/cmd/spire-server/cli/run/run.go +++ b/cmd/spire-server/cli/run/run.go @@ -389,6 +389,7 @@ func NewServerConfig(c *Config, logOptions []log.Option, allowUnknownConfig bool if err != nil { return nil, fmt.Errorf("could not start logger: %w", err) } + sc.Logger = logger sc.Log = logger if reopenableFile != nil { sc.LogReopener = log.ReopenOnSignal(logger, reopenableFile) diff --git a/pkg/server/api/logger/v1/service.go b/pkg/server/api/logger/v1/service.go index a8cb3e8338..cb923ca512 100644 --- a/pkg/server/api/logger/v1/service.go +++ b/pkg/server/api/logger/v1/service.go @@ -5,6 +5,7 @@ import ( "context" "strings" + "github.com/spiffe/spire/pkg/common/log" loggerv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/logger/v1" apitype "github.com/spiffe/spire-api-sdk/proto/spire/api/types" @@ -14,20 +15,24 @@ import ( type Config struct { LaunchLevel logrus.Level + Logger *log.Logger } type Service struct { loggerv1.UnsafeLoggerServer LaunchLevel logrus.Level + Logger *log.Logger } func New(config Config) *Service { logrus.WithFields(logrus.Fields{ "LaunchLevel": config.LaunchLevel, + "LoggerAddress": config.Logger, }).Info("logger service Configured") return &Service{ LaunchLevel: config.LaunchLevel, + Logger: config.Logger, } } @@ -38,7 +43,7 @@ func RegisterService(server *grpc.Server, service *Service) { func (service *Service) GetLogger(ctx context.Context, req *loggerv1.GetLoggerRequest) (*apitype.Logger, error) { logrus.Info("GetLogger Called") logger := &apitype.Logger{ - CurrentLevel: ApiLevel[logrus.GetLevel()], + CurrentLevel: ApiLevel[service.Logger.GetLevel()], LaunchLevel: ApiLevel[service.LaunchLevel], } return logger, nil @@ -52,9 +57,9 @@ func (service *Service) SetLogLevel(ctx context.Context, req *loggerv1.SetLogLev "ApiLogLevel": strings.ToLower(apitype.LogLevel_name[int32(req.NewLevel)]), "LogrusLevel": LogrusLevel[req.NewLevel].String(), }).Info("SetLogLevel Called") - logrus.SetLevel(LogrusLevel[req.NewLevel]) + service.Logger.SetLevel(LogrusLevel[req.NewLevel]) logger := &apitype.Logger{ - CurrentLevel: ApiLevel[logrus.GetLevel()], + CurrentLevel: ApiLevel[service.Logger.GetLevel()], LaunchLevel: ApiLevel[service.LaunchLevel], } return logger, nil diff --git a/pkg/server/config.go b/pkg/server/config.go index cf9eec8687..867a39eb5e 100644 --- a/pkg/server/config.go +++ b/pkg/server/config.go @@ -16,6 +16,7 @@ import ( "github.com/spiffe/spire/pkg/server/endpoints" "github.com/spiffe/spire/pkg/server/endpoints/bundle" "github.com/spiffe/spire/pkg/server/plugin/keymanager" + "github.com/spiffe/spire/pkg/common/log" ) type Config struct { @@ -24,6 +25,8 @@ type Config struct { Log logrus.FieldLogger + Logger *log.Logger + LaunchLogLevel logrus.Level // LogReopener facilitates handling a signal to rotate log file. diff --git a/pkg/server/endpoints/config.go b/pkg/server/endpoints/config.go index af6c4cd3fa..459f469255 100644 --- a/pkg/server/endpoints/config.go +++ b/pkg/server/endpoints/config.go @@ -29,6 +29,7 @@ import ( "github.com/spiffe/spire/pkg/server/ca/manager" "github.com/spiffe/spire/pkg/server/cache/dscache" "github.com/spiffe/spire/pkg/server/catalog" + "github.com/spiffe/spire/pkg/common/log" "github.com/spiffe/spire/pkg/server/endpoints/bundle" "github.com/spiffe/spire/pkg/server/svid" ) @@ -62,9 +63,12 @@ type Config struct { // Makes policy decisions AuthPolicyEngine *authpolicy.Engine - // The fielLogger interface + // The fieldLogger interface Log logrus.FieldLogger + // The logger instance (permits log level changes) + Logger *log.Logger + // The default (original config) log level LaunchLogLevel logrus.Level @@ -165,6 +169,7 @@ func (c *Config) makeAPIServers(entryFetcher api.AuthorizedEntryFetcher) APIServ }), LoggerServer: loggerv1.New(loggerv1.Config{ LaunchLevel: c.LaunchLogLevel, + Logger: c.Logger, }), SVIDServer: svidv1.New(svidv1.Config{ TrustDomain: c.TrustDomain, diff --git a/pkg/server/server.go b/pkg/server/server.go index 4eaa4cab9d..09a1903cac 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -390,6 +390,7 @@ func (s *Server) newEndpointsServer(ctx context.Context, catalog catalog.Catalog Catalog: catalog, ServerCA: serverCA, Log: s.config.Log.WithField(telemetry.SubsystemName, telemetry.Endpoints), + Logger: s.config.Logger, LaunchLogLevel: s.config.LaunchLogLevel, Metrics: metrics, JWTKeyPublisher: jwtKeyPublisher, From 6d912a83aa2fc2e6810454b79b5f314d2de108e7 Mon Sep 17 00:00:00 2001 From: Edwin Buck Date: Mon, 4 Mar 2024 18:52:25 -0600 Subject: [PATCH 09/20] Make the service fields private, small renames, add a type. The Logger service fields are now private, the service config fields remain public. The Logger variable / field names are now log (or Log) to match conventions. The logger package now adds a Logger type which extends the logrus.Logger interface with set log level and get log level funcs. Signed-off-by: Edwin Buck --- cmd/spire-server/cli/run/run.go | 1 - pkg/server/api/logger/v1/service.go | 46 ++++++++++++++++------------- pkg/server/config.go | 6 ++-- pkg/server/endpoints/config.go | 13 ++++---- pkg/server/server.go | 4 +-- 5 files changed, 35 insertions(+), 35 deletions(-) diff --git a/cmd/spire-server/cli/run/run.go b/cmd/spire-server/cli/run/run.go index acd72e36a2..0dbab00327 100644 --- a/cmd/spire-server/cli/run/run.go +++ b/cmd/spire-server/cli/run/run.go @@ -389,7 +389,6 @@ func NewServerConfig(c *Config, logOptions []log.Option, allowUnknownConfig bool if err != nil { return nil, fmt.Errorf("could not start logger: %w", err) } - sc.Logger = logger sc.Log = logger if reopenableFile != nil { sc.LogReopener = log.ReopenOnSignal(logger, reopenableFile) diff --git a/pkg/server/api/logger/v1/service.go b/pkg/server/api/logger/v1/service.go index cb923ca512..daf48a7e10 100644 --- a/pkg/server/api/logger/v1/service.go +++ b/pkg/server/api/logger/v1/service.go @@ -1,11 +1,10 @@ package logger import ( - "fmt" "context" + "fmt" "strings" - "github.com/spiffe/spire/pkg/common/log" loggerv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/logger/v1" apitype "github.com/spiffe/spire-api-sdk/proto/spire/api/types" @@ -13,26 +12,31 @@ import ( "google.golang.org/grpc" ) +type Logger interface { + logrus.FieldLogger + GetLevel() logrus.Level + SetLevel(level logrus.Level) +} + type Config struct { + Log Logger LaunchLevel logrus.Level - Logger *log.Logger } type Service struct { loggerv1.UnsafeLoggerServer - LaunchLevel logrus.Level - Logger *log.Logger + log Logger + launchLevel logrus.Level } func New(config Config) *Service { - logrus.WithFields(logrus.Fields{ + config.Log.WithFields(logrus.Fields{ "LaunchLevel": config.LaunchLevel, - "LoggerAddress": config.Logger, - }).Info("logger service Configured") + }).Info("Logger service Configured") return &Service{ - LaunchLevel: config.LaunchLevel, - Logger: config.Logger, + log: config.Log, + launchLevel: config.LaunchLevel, } } @@ -41,10 +45,10 @@ func RegisterService(server *grpc.Server, service *Service) { } func (service *Service) GetLogger(ctx context.Context, req *loggerv1.GetLoggerRequest) (*apitype.Logger, error) { - logrus.Info("GetLogger Called") + service.log.Info("GetLogger Called") logger := &apitype.Logger{ - CurrentLevel: ApiLevel[service.Logger.GetLevel()], - LaunchLevel: ApiLevel[service.LaunchLevel], + CurrentLevel: ApiLevel[service.log.GetLevel()], + LaunchLevel: ApiLevel[service.launchLevel], } return logger, nil } @@ -53,24 +57,24 @@ func (service *Service) SetLogLevel(ctx context.Context, req *loggerv1.SetLogLev if req.NewLevel == apitype.LogLevel_UNSPECIFIED { return nil, fmt.Errorf("Invalid request NewLevel value cannot be LogLevel_UNSPECIFIED") } - logrus.WithFields(logrus.Fields{ + service.log.WithFields(logrus.Fields{ "ApiLogLevel": strings.ToLower(apitype.LogLevel_name[int32(req.NewLevel)]), "LogrusLevel": LogrusLevel[req.NewLevel].String(), }).Info("SetLogLevel Called") - service.Logger.SetLevel(LogrusLevel[req.NewLevel]) + service.log.SetLevel(LogrusLevel[req.NewLevel]) logger := &apitype.Logger{ - CurrentLevel: ApiLevel[service.Logger.GetLevel()], - LaunchLevel: ApiLevel[service.LaunchLevel], + CurrentLevel: ApiLevel[service.log.GetLevel()], + LaunchLevel: ApiLevel[service.launchLevel], } return logger, nil } func (service *Service) ResetLogLevel(ctx context.Context, req *loggerv1.ResetLogLevelRequest) (*apitype.Logger, error) { - logrus.Info("ResetLogLevel Called") - logrus.SetLevel(service.LaunchLevel) + service.log.Info("ResetLogLevel Called") + service.log.SetLevel(service.launchLevel) logger := &apitype.Logger{ - CurrentLevel: ApiLevel[logrus.GetLevel()], - LaunchLevel: ApiLevel[service.LaunchLevel], + CurrentLevel: ApiLevel[service.log.GetLevel()], + LaunchLevel: ApiLevel[service.launchLevel], } return logger, nil } diff --git a/pkg/server/config.go b/pkg/server/config.go index 867a39eb5e..c973151b56 100644 --- a/pkg/server/config.go +++ b/pkg/server/config.go @@ -11,21 +11,19 @@ import ( common "github.com/spiffe/spire/pkg/common/catalog" "github.com/spiffe/spire/pkg/common/health" "github.com/spiffe/spire/pkg/common/telemetry" + loggerv1 "github.com/spiffe/spire/pkg/server/api/logger/v1" "github.com/spiffe/spire/pkg/server/authpolicy" bundle_client "github.com/spiffe/spire/pkg/server/bundle/client" "github.com/spiffe/spire/pkg/server/endpoints" "github.com/spiffe/spire/pkg/server/endpoints/bundle" "github.com/spiffe/spire/pkg/server/plugin/keymanager" - "github.com/spiffe/spire/pkg/common/log" ) type Config struct { // Configurations for server plugins PluginConfigs common.PluginConfigs - Log logrus.FieldLogger - - Logger *log.Logger + Log loggerv1.Logger LaunchLogLevel logrus.Level diff --git a/pkg/server/endpoints/config.go b/pkg/server/endpoints/config.go index 459f469255..4447c7ed7b 100644 --- a/pkg/server/endpoints/config.go +++ b/pkg/server/endpoints/config.go @@ -19,8 +19,8 @@ import ( bundlev1 "github.com/spiffe/spire/pkg/server/api/bundle/v1" debugv1 "github.com/spiffe/spire/pkg/server/api/debug/v1" entryv1 "github.com/spiffe/spire/pkg/server/api/entry/v1" - loggerv1 "github.com/spiffe/spire/pkg/server/api/logger/v1" healthv1 "github.com/spiffe/spire/pkg/server/api/health/v1" + loggerv1 "github.com/spiffe/spire/pkg/server/api/logger/v1" svidv1 "github.com/spiffe/spire/pkg/server/api/svid/v1" trustdomainv1 "github.com/spiffe/spire/pkg/server/api/trustdomain/v1" "github.com/spiffe/spire/pkg/server/authpolicy" @@ -29,7 +29,6 @@ import ( "github.com/spiffe/spire/pkg/server/ca/manager" "github.com/spiffe/spire/pkg/server/cache/dscache" "github.com/spiffe/spire/pkg/server/catalog" - "github.com/spiffe/spire/pkg/common/log" "github.com/spiffe/spire/pkg/server/endpoints/bundle" "github.com/spiffe/spire/pkg/server/svid" ) @@ -63,11 +62,11 @@ type Config struct { // Makes policy decisions AuthPolicyEngine *authpolicy.Engine - // The fieldLogger interface - Log logrus.FieldLogger + // The logger for the endpoints subsystem + Log logrus.FieldLogger - // The logger instance (permits log level changes) - Logger *log.Logger + // The root logger for the entire process + RootLog loggerv1.Logger // The default (original config) log level LaunchLogLevel logrus.Level @@ -168,8 +167,8 @@ func (c *Config) makeAPIServers(entryFetcher api.AuthorizedEntryFetcher) APIServ DataStore: ds, }), LoggerServer: loggerv1.New(loggerv1.Config{ + Log: c.RootLog, LaunchLevel: c.LaunchLogLevel, - Logger: c.Logger, }), SVIDServer: svidv1.New(svidv1.Config{ TrustDomain: c.TrustDomain, diff --git a/pkg/server/server.go b/pkg/server/server.go index 09a1903cac..304736e255 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -390,8 +390,8 @@ func (s *Server) newEndpointsServer(ctx context.Context, catalog catalog.Catalog Catalog: catalog, ServerCA: serverCA, Log: s.config.Log.WithField(telemetry.SubsystemName, telemetry.Endpoints), - Logger: s.config.Logger, - LaunchLogLevel: s.config.LaunchLogLevel, + RootLog: s.config.Log, + LaunchLogLevel: s.config.LaunchLogLevel, Metrics: metrics, JWTKeyPublisher: jwtKeyPublisher, RateLimit: s.config.RateLimit, From 0355b90e9aaed30dc7b21910b96e3eb5554000bd Mon Sep 17 00:00:00 2001 From: Edwin Buck Date: Mon, 4 Mar 2024 19:00:38 -0600 Subject: [PATCH 10/20] Add the RootLog field to the config in the endpoints_test.go. This fixes the inability of this unit test to intialize the new GRPC logger service, as it requires a Logger type that also includes the ability to change log levels. Signed-off-by: Edwin Buck --- pkg/server/endpoints/endpoints_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/server/endpoints/endpoints_test.go b/pkg/server/endpoints/endpoints_test.go index 812ff6ee5d..1e42828742 100644 --- a/pkg/server/endpoints/endpoints_test.go +++ b/pkg/server/endpoints/endpoints_test.go @@ -95,6 +95,7 @@ func TestNew(t *testing.T) { BundleEndpoint: bundle.EndpointConfig{Address: tcpAddr}, JWTKeyPublisher: &fakeJWTKeyPublisher{}, Log: log, + RootLog: log, Metrics: metrics, RateLimit: rateLimit, Clock: clk, From 5aa92ffa929519d63023cf3bda3e3928a0f327a1 Mon Sep 17 00:00:00 2001 From: Edwin Buck Date: Mon, 4 Mar 2024 19:57:24 -0600 Subject: [PATCH 11/20] Minor adjustements to conform with go's linting policies. Signed-off-by: Edwin Buck --- cmd/spire-server/cli/logger/get.go | 12 +++---- cmd/spire-server/cli/logger/get_test.go | 33 ++++++++++---------- cmd/spire-server/cli/logger/mocks_test.go | 19 ++++++----- cmd/spire-server/cli/logger/printers.go | 9 +++--- cmd/spire-server/cli/logger/printers_test.go | 27 ++++++++-------- cmd/spire-server/cli/logger/set.go | 24 +++++++------- cmd/spire-server/cli/logger/set_test.go | 33 ++++++++++---------- pkg/server/api/logger/v1/levels.go | 11 +++---- pkg/server/api/logger/v1/service.go | 22 ++++++------- pkg/server/endpoints/config.go | 2 +- 10 files changed, 94 insertions(+), 98 deletions(-) diff --git a/cmd/spire-server/cli/logger/get.go b/cmd/spire-server/cli/logger/get.go index 6c359328ee..b6753a3df6 100644 --- a/cmd/spire-server/cli/logger/get.go +++ b/cmd/spire-server/cli/logger/get.go @@ -13,8 +13,8 @@ import ( ) type getCommand struct { - env *commoncli.Env - printer cliprinter.Printer + env *commoncli.Env + printer cliprinter.Printer } // Returns a cli.command that gets the logger information using @@ -29,12 +29,12 @@ func NewGetCommandWithEnv(env *commoncli.Env) cli.Command { } // The name of the command. -func (_ *getCommand) Name() string { +func (*getCommand) Name() string { return "logger get" } // The help presented description of the command. -func (_ *getCommand) Synopsis() string { +func (*getCommand) Synopsis() string { return "Gets the logger details" } @@ -45,7 +45,6 @@ func (c *getCommand) AppendFlags(fs *flag.FlagSet) { // The routine that executes the command func (c *getCommand) Run(ctx context.Context, _ *commoncli.Env, serverClient util.ServerClient) error { - logger, err := serverClient.NewLoggerClient().GetLogger(ctx, &api.GetLoggerRequest{}) if err != nil { return fmt.Errorf("error fetching logger: %w", err) @@ -55,7 +54,6 @@ func (c *getCommand) Run(ctx context.Context, _ *commoncli.Env, serverClient uti } // Formatting for the logger under pretty printing of output. -func (l* getCommand) prettyPrintLogger(env *commoncli.Env, results ...any) error { +func (c *getCommand) prettyPrintLogger(env *commoncli.Env, results ...any) error { return PrettyPrintLogger(env, results...) } - diff --git a/cmd/spire-server/cli/logger/get_test.go b/cmd/spire-server/cli/logger/get_test.go index 13794fe6f5..002df8602b 100644 --- a/cmd/spire-server/cli/logger/get_test.go +++ b/cmd/spire-server/cli/logger/get_test.go @@ -3,10 +3,11 @@ package logger_test import ( "errors" "testing" + "github.com/stretchr/testify/require" - "github.com/spiffe/spire/cmd/spire-server/cli/logger" "github.com/spiffe/spire-api-sdk/proto/spire/api/types" + "github.com/spiffe/spire/cmd/spire-server/cli/logger" ) var ( @@ -31,16 +32,16 @@ func TestGetSynopsis(t *testing.T) { } func TestGet(t *testing.T) { - for _, tt := range []struct{ - name string + for _, tt := range []struct { + name string // server state - server *mockLoggerServer + server *mockLoggerServer // input - args []string + args []string // expected items - expectReturnCode int - expectStdout string - expectStderr string + expectReturnCode int + expectStdout string + expectStderr string }{ { name: "configured to info, set to info, using pretty output", @@ -48,7 +49,7 @@ func TestGet(t *testing.T) { server: &mockLoggerServer{ returnLogger: &types.Logger{ CurrentLevel: types.LogLevel_INFO, - LaunchLevel: types.LogLevel_INFO, + LaunchLevel: types.LogLevel_INFO, }, }, expectReturnCode: 0, @@ -63,7 +64,7 @@ Launch Level : info server: &mockLoggerServer{ returnLogger: &types.Logger{ CurrentLevel: types.LogLevel_WARN, - LaunchLevel: types.LogLevel_DEBUG, + LaunchLevel: types.LogLevel_DEBUG, }, }, expectReturnCode: 0, @@ -78,7 +79,7 @@ Launch Level : debug server: &mockLoggerServer{ returnLogger: &types.Logger{ CurrentLevel: types.LogLevel_TRACE, - LaunchLevel: types.LogLevel_ERROR, + LaunchLevel: types.LogLevel_ERROR, }, }, expectReturnCode: 0, @@ -93,7 +94,7 @@ Launch Level : error server: &mockLoggerServer{ returnLogger: &types.Logger{ CurrentLevel: types.LogLevel_FATAL, - LaunchLevel: types.LogLevel_PANIC, + LaunchLevel: types.LogLevel_PANIC, }, }, expectReturnCode: 0, @@ -108,7 +109,7 @@ Launch Level : panic server: &mockLoggerServer{ returnLogger: &types.Logger{ CurrentLevel: types.LogLevel_INFO, - LaunchLevel: types.LogLevel_INFO, + LaunchLevel: types.LogLevel_INFO, }, }, expectReturnCode: 0, @@ -121,7 +122,7 @@ Launch Level : panic server: &mockLoggerServer{ returnLogger: &types.Logger{ CurrentLevel: types.LogLevel_WARN, - LaunchLevel: types.LogLevel_DEBUG, + LaunchLevel: types.LogLevel_DEBUG, }, }, expectReturnCode: 0, @@ -134,7 +135,7 @@ Launch Level : panic server: &mockLoggerServer{ returnLogger: &types.Logger{ CurrentLevel: types.LogLevel_TRACE, - LaunchLevel: types.LogLevel_ERROR, + LaunchLevel: types.LogLevel_ERROR, }, }, expectReturnCode: 0, @@ -147,7 +148,7 @@ Launch Level : panic server: &mockLoggerServer{ returnLogger: &types.Logger{ CurrentLevel: types.LogLevel_FATAL, - LaunchLevel: types.LogLevel_PANIC, + LaunchLevel: types.LogLevel_PANIC, }, }, expectReturnCode: 0, diff --git a/cmd/spire-server/cli/logger/mocks_test.go b/cmd/spire-server/cli/logger/mocks_test.go index a4a120c4c1..80eeba4a52 100644 --- a/cmd/spire-server/cli/logger/mocks_test.go +++ b/cmd/spire-server/cli/logger/mocks_test.go @@ -3,17 +3,18 @@ package logger_test import ( "io" "testing" + "github.com/spiffe/spire/test/spiretest" "bytes" "context" "github.com/mitchellh/cli" + loggerv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/logger/v1" + "github.com/spiffe/spire-api-sdk/proto/spire/api/types" "github.com/spiffe/spire/cmd/spire-server/cli/common" commoncli "github.com/spiffe/spire/pkg/common/cli" "google.golang.org/grpc" - loggerv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/logger/v1" - "github.com/spiffe/spire-api-sdk/proto/spire/api/types" ) // an input/output capture struct @@ -22,7 +23,7 @@ type loggerTest struct { stdout *bytes.Buffer stderr *bytes.Buffer args []string - server *mockLoggerServer + server *mockLoggerServer client cli.Command } @@ -40,7 +41,7 @@ func setupCliTest(t *testing.T, server *mockLoggerServer, newClient func(*common loggerv1.RegisterLoggerServer(s, server) }) - stdin := new(bytes.Buffer) + stdin := new(bytes.Buffer) stdout := new(bytes.Buffer) stderr := new(bytes.Buffer) @@ -71,8 +72,8 @@ type mockLoggerServer struct { loggerv1.UnimplementedLoggerServer receivedSetValue *types.LogLevel - returnLogger *types.Logger - returnErr error + returnLogger *types.Logger + returnErr error } // mock implementation for GetLogger @@ -85,17 +86,16 @@ func (s *mockLoggerServer) SetLogLevel(_ context.Context, req *loggerv1.SetLogLe return s.returnLogger, s.returnErr } -func (s *mockLoggerServer) ResetLogLevel(_ context.Context, req *loggerv1.ResetLogLevelRequest) (*types.Logger, error) { +func (s *mockLoggerServer) ResetLogLevel(_ context.Context, _ *loggerv1.ResetLogLevelRequest) (*types.Logger, error) { s.receivedSetValue = nil return s.returnLogger, s.returnErr } - var _ io.Writer = &errorWriter{} type errorWriter struct { ReturnError error - Buffer bytes.Buffer + Buffer bytes.Buffer } func (e *errorWriter) Write(p []byte) (n int, err error) { @@ -108,4 +108,3 @@ func (e *errorWriter) Write(p []byte) (n int, err error) { func (e *errorWriter) String() string { return e.Buffer.String() } - diff --git a/cmd/spire-server/cli/logger/printers.go b/cmd/spire-server/cli/logger/printers.go index 70d9dddf01..c88a7c91cc 100644 --- a/cmd/spire-server/cli/logger/printers.go +++ b/cmd/spire-server/cli/logger/printers.go @@ -17,20 +17,20 @@ func PrettyPrintLogger(env *commoncli.Env, results ...any) error { logrusCurrent, found := serverlogger.LogrusLevel[apiLogger.CurrentLevel] if !found { - return errors.New("internal error: returned current log level is undefined; please report this as a bug.") + return errors.New("internal error: returned current log level is undefined; please report this as a bug") } currentText, err := logrusCurrent.MarshalText() if err != nil { - return fmt.Errorf("internal error: logrus log level %d has no name; please report this as a bug.", logrusCurrent) + return fmt.Errorf("internal error: logrus log level %d has no name; please report this as a bug", logrusCurrent) } logrusLaunch, found := serverlogger.LogrusLevel[apiLogger.LaunchLevel] if !found { - return errors.New("internal error: returned current log level is undefined; please report this as a bug.") + return errors.New("internal error: returned current log level is undefined; please report this as a bug") } launchText, err := logrusLaunch.MarshalText() if err != nil { - return fmt.Errorf("internal error: logrus log level %d has no name; please report this as a bug.", logrusLaunch) + return fmt.Errorf("internal error: logrus log level %d has no name; please report this as a bug", logrusLaunch) } if err := env.Printf("Logger Level : %s\nLaunch Level : %s\n\n", currentText, launchText); err != nil { @@ -38,4 +38,3 @@ func PrettyPrintLogger(env *commoncli.Env, results ...any) error { } return nil } - diff --git a/cmd/spire-server/cli/logger/printers_test.go b/cmd/spire-server/cli/logger/printers_test.go index 32b760dc68..cd5df7d3c9 100644 --- a/cmd/spire-server/cli/logger/printers_test.go +++ b/cmd/spire-server/cli/logger/printers_test.go @@ -3,29 +3,30 @@ package logger_test import ( "errors" "testing" + "github.com/stretchr/testify/require" - "github.com/spiffe/spire/cmd/spire-server/cli/logger" "github.com/spiffe/spire-api-sdk/proto/spire/api/types" + "github.com/spiffe/spire/cmd/spire-server/cli/logger" commoncli "github.com/spiffe/spire/pkg/common/cli" ) func TestPrettyPrintLogger(t *testing.T) { for _, tt := range []struct { - name string - logger interface{} - outWriter errorWriter - errWriter errorWriter - env *commoncli.Env + name string + logger interface{} + outWriter errorWriter + errWriter errorWriter + env *commoncli.Env expectedStdout string expectedStderr string - expectedError error + expectedError error }{ { name: "test", logger: &types.Logger{ CurrentLevel: types.LogLevel_DEBUG, - LaunchLevel: types.LogLevel_INFO, + LaunchLevel: types.LogLevel_INFO, }, expectedStdout: `Logger Level : debug Launch Level : info @@ -39,7 +40,7 @@ Launch Level : info }, logger: &types.Logger{ CurrentLevel: types.LogLevel_DEBUG, - LaunchLevel: types.LogLevel_INFO, + LaunchLevel: types.LogLevel_INFO, }, expectedError: errors.New("cannot write"), }, @@ -48,11 +49,11 @@ Launch Level : info outWriter: errorWriter{ ReturnError: errors.New("cannot write"), }, - logger: &types.Entry{ - }, + logger: &types.Entry{}, expectedError: errors.New("internal error: logger not found; please report this as a bug"), }, - }{ + } { + tt := tt t.Run(tt.name, func(t *testing.T) { tt.env = &commoncli.Env{ Stdout: &tt.outWriter, @@ -63,6 +64,4 @@ Launch Level : info require.Equal(t, tt.errWriter.String(), tt.expectedStderr) }) } - } - diff --git a/cmd/spire-server/cli/logger/set.go b/cmd/spire-server/cli/logger/set.go index 23da518028..d43220380e 100644 --- a/cmd/spire-server/cli/logger/set.go +++ b/cmd/spire-server/cli/logger/set.go @@ -7,23 +7,23 @@ import ( "strings" "github.com/mitchellh/cli" + "github.com/sirupsen/logrus" api "github.com/spiffe/spire-api-sdk/proto/spire/api/server/logger/v1" apitype "github.com/spiffe/spire-api-sdk/proto/spire/api/types" - "github.com/sirupsen/logrus" "github.com/spiffe/spire/cmd/spire-server/util" - serverlogger "github.com/spiffe/spire/pkg/server/api/logger/v1" commoncli "github.com/spiffe/spire/pkg/common/cli" "github.com/spiffe/spire/pkg/common/cliprinter" + serverlogger "github.com/spiffe/spire/pkg/server/api/logger/v1" ) type setCommand struct { - env *commoncli.Env - newLevel string - printer cliprinter.Printer + env *commoncli.Env + newLevel string + printer cliprinter.Printer } // Returns a cli.command that sets the log level using the default -// cli enviornment. +// cli environment. func NewSetCommand() cli.Command { return NewSetCommandWithEnv(commoncli.DefaultEnv) } @@ -34,12 +34,12 @@ func NewSetCommandWithEnv(env *commoncli.Env) cli.Command { } // The name of the command. -func (_ *setCommand) Name() string { +func (*setCommand) Name() string { return "logger set" } // The help presented description of the command. -func (_ *setCommand) Synopsis() string { +func (*setCommand) Synopsis() string { return "Sets the logger details" } @@ -49,7 +49,6 @@ func (c *setCommand) AppendFlags(fs *flag.FlagSet) { cliprinter.AppendFlagWithCustomPretty(&c.printer, fs, c.env, c.prettyPrintLogger) } - // The routine that executes the command func (c *setCommand) Run(ctx context.Context, _ *commoncli.Env, serverClient util.ServerClient) error { if c.newLevel == "" { @@ -61,11 +60,12 @@ func (c *setCommand) Run(ctx context.Context, _ *commoncli.Env, serverClient uti if level == "launch" { logger, err = serverClient.NewLoggerClient().ResetLogLevel(ctx, &api.ResetLogLevelRequest{}) } else { - logrusLevel, err := logrus.ParseLevel(level) + var logrusLevel logrus.Level + logrusLevel, err = logrus.ParseLevel(level) if err != nil { return fmt.Errorf("the value %s is not a valid setting", c.newLevel) } - apiLevel, found := serverlogger.ApiLevel[logrusLevel] + apiLevel, found := serverlogger.APILevel[logrusLevel] if !found { return fmt.Errorf("the logrus level %d could not be transformed into an api log level", logrusLevel) } @@ -79,6 +79,6 @@ func (c *setCommand) Run(ctx context.Context, _ *commoncli.Env, serverClient uti return c.printer.PrintProto(logger) } -func (l* setCommand) prettyPrintLogger(env *commoncli.Env, results ...any) error { +func (c *setCommand) prettyPrintLogger(env *commoncli.Env, results ...any) error { return PrettyPrintLogger(env, results...) } diff --git a/cmd/spire-server/cli/logger/set_test.go b/cmd/spire-server/cli/logger/set_test.go index 5ced0bc164..36aece3f16 100644 --- a/cmd/spire-server/cli/logger/set_test.go +++ b/cmd/spire-server/cli/logger/set_test.go @@ -2,10 +2,11 @@ package logger_test import ( "testing" + "github.com/stretchr/testify/require" - "github.com/spiffe/spire/cmd/spire-server/cli/logger" "github.com/spiffe/spire-api-sdk/proto/spire/api/types" + "github.com/spiffe/spire/cmd/spire-server/cli/logger" ) var ( @@ -32,17 +33,17 @@ func TestSetSynopsis(t *testing.T) { } func TestSet(t *testing.T) { - for _, tt := range []struct{ - name string + for _, tt := range []struct { + name string // server state - server *mockLoggerServer + server *mockLoggerServer // input - args []string + args []string // expected items - expectedSetValue types.LogLevel - expectReturnCode int - expectStdout string - expectStderr string + expectedSetValue types.LogLevel + expectReturnCode int + expectStdout string + expectStderr string }{ { name: "set to debug, configured to info, using pretty output", @@ -50,7 +51,7 @@ func TestSet(t *testing.T) { server: &mockLoggerServer{ returnLogger: &types.Logger{ CurrentLevel: types.LogLevel_DEBUG, - LaunchLevel: types.LogLevel_INFO, + LaunchLevel: types.LogLevel_INFO, }, }, expectedSetValue: types.LogLevel_DEBUG, @@ -66,7 +67,7 @@ Launch Level : info server: &mockLoggerServer{ returnLogger: &types.Logger{ CurrentLevel: types.LogLevel_WARN, - LaunchLevel: types.LogLevel_DEBUG, + LaunchLevel: types.LogLevel_DEBUG, }, }, expectedSetValue: types.LogLevel_WARN, @@ -82,7 +83,7 @@ Launch Level : debug server: &mockLoggerServer{ returnLogger: &types.Logger{ CurrentLevel: types.LogLevel_ERROR, - LaunchLevel: types.LogLevel_ERROR, + LaunchLevel: types.LogLevel_ERROR, }, }, expectReturnCode: 0, @@ -97,7 +98,7 @@ Launch Level : error server: &mockLoggerServer{ returnLogger: &types.Logger{ CurrentLevel: types.LogLevel_PANIC, - LaunchLevel: types.LogLevel_FATAL, + LaunchLevel: types.LogLevel_FATAL, }, }, expectedSetValue: types.LogLevel_PANIC, @@ -113,7 +114,7 @@ Launch Level : fatal server: &mockLoggerServer{ returnLogger: &types.Logger{ CurrentLevel: types.LogLevel_INFO, - LaunchLevel: types.LogLevel_INFO, + LaunchLevel: types.LogLevel_INFO, }, }, expectReturnCode: 1, @@ -126,7 +127,7 @@ Launch Level : fatal server: &mockLoggerServer{ returnLogger: &types.Logger{ CurrentLevel: types.LogLevel_INFO, - LaunchLevel: types.LogLevel_INFO, + LaunchLevel: types.LogLevel_INFO, }, }, expectReturnCode: 1, @@ -139,7 +140,7 @@ Launch Level : fatal server: &mockLoggerServer{ returnLogger: &types.Logger{ CurrentLevel: types.LogLevel_INFO, - LaunchLevel: types.LogLevel_INFO, + LaunchLevel: types.LogLevel_INFO, }, }, expectedSetValue: types.LogLevel_TRACE, diff --git a/pkg/server/api/logger/v1/levels.go b/pkg/server/api/logger/v1/levels.go index bcf99339ba..c8f0c06dcc 100644 --- a/pkg/server/api/logger/v1/levels.go +++ b/pkg/server/api/logger/v1/levels.go @@ -5,12 +5,12 @@ import ( apitype "github.com/spiffe/spire-api-sdk/proto/spire/api/types" ) -var ApiLevel = map[logrus.Level]apitype.LogLevel{ +var APILevel = map[logrus.Level]apitype.LogLevel{ logrus.PanicLevel: apitype.LogLevel_PANIC, logrus.FatalLevel: apitype.LogLevel_FATAL, logrus.ErrorLevel: apitype.LogLevel_ERROR, - logrus.WarnLevel: apitype.LogLevel_WARN, - logrus.InfoLevel: apitype.LogLevel_INFO, + logrus.WarnLevel: apitype.LogLevel_WARN, + logrus.InfoLevel: apitype.LogLevel_INFO, logrus.DebugLevel: apitype.LogLevel_DEBUG, logrus.TraceLevel: apitype.LogLevel_TRACE, } @@ -19,9 +19,8 @@ var LogrusLevel = map[apitype.LogLevel]logrus.Level{ apitype.LogLevel_PANIC: logrus.PanicLevel, apitype.LogLevel_FATAL: logrus.FatalLevel, apitype.LogLevel_ERROR: logrus.ErrorLevel, - apitype.LogLevel_WARN: logrus.WarnLevel, - apitype.LogLevel_INFO: logrus.InfoLevel, + apitype.LogLevel_WARN: logrus.WarnLevel, + apitype.LogLevel_INFO: logrus.InfoLevel, apitype.LogLevel_DEBUG: logrus.DebugLevel, apitype.LogLevel_TRACE: logrus.TraceLevel, } - diff --git a/pkg/server/api/logger/v1/service.go b/pkg/server/api/logger/v1/service.go index daf48a7e10..da35040b9f 100644 --- a/pkg/server/api/logger/v1/service.go +++ b/pkg/server/api/logger/v1/service.go @@ -19,14 +19,14 @@ type Logger interface { } type Config struct { - Log Logger + Log Logger LaunchLevel logrus.Level } type Service struct { loggerv1.UnsafeLoggerServer - log Logger + log Logger launchLevel logrus.Level } @@ -44,16 +44,16 @@ func RegisterService(server *grpc.Server, service *Service) { loggerv1.RegisterLoggerServer(server, service) } -func (service *Service) GetLogger(ctx context.Context, req *loggerv1.GetLoggerRequest) (*apitype.Logger, error) { +func (service *Service) GetLogger(_ context.Context, _ *loggerv1.GetLoggerRequest) (*apitype.Logger, error) { service.log.Info("GetLogger Called") logger := &apitype.Logger{ - CurrentLevel: ApiLevel[service.log.GetLevel()], - LaunchLevel: ApiLevel[service.launchLevel], + CurrentLevel: APILevel[service.log.GetLevel()], + LaunchLevel: APILevel[service.launchLevel], } return logger, nil } -func (service *Service) SetLogLevel(ctx context.Context, req *loggerv1.SetLogLevelRequest) (*apitype.Logger, error) { +func (service *Service) SetLogLevel(_ context.Context, req *loggerv1.SetLogLevelRequest) (*apitype.Logger, error) { if req.NewLevel == apitype.LogLevel_UNSPECIFIED { return nil, fmt.Errorf("Invalid request NewLevel value cannot be LogLevel_UNSPECIFIED") } @@ -63,18 +63,18 @@ func (service *Service) SetLogLevel(ctx context.Context, req *loggerv1.SetLogLev }).Info("SetLogLevel Called") service.log.SetLevel(LogrusLevel[req.NewLevel]) logger := &apitype.Logger{ - CurrentLevel: ApiLevel[service.log.GetLevel()], - LaunchLevel: ApiLevel[service.launchLevel], + CurrentLevel: APILevel[service.log.GetLevel()], + LaunchLevel: APILevel[service.launchLevel], } return logger, nil } -func (service *Service) ResetLogLevel(ctx context.Context, req *loggerv1.ResetLogLevelRequest) (*apitype.Logger, error) { +func (service *Service) ResetLogLevel(_ context.Context, _ *loggerv1.ResetLogLevelRequest) (*apitype.Logger, error) { service.log.Info("ResetLogLevel Called") service.log.SetLevel(service.launchLevel) logger := &apitype.Logger{ - CurrentLevel: ApiLevel[service.log.GetLevel()], - LaunchLevel: ApiLevel[service.launchLevel], + CurrentLevel: APILevel[service.log.GetLevel()], + LaunchLevel: APILevel[service.launchLevel], } return logger, nil } diff --git a/pkg/server/endpoints/config.go b/pkg/server/endpoints/config.go index 4447c7ed7b..20d205a3ff 100644 --- a/pkg/server/endpoints/config.go +++ b/pkg/server/endpoints/config.go @@ -167,7 +167,7 @@ func (c *Config) makeAPIServers(entryFetcher api.AuthorizedEntryFetcher) APIServ DataStore: ds, }), LoggerServer: loggerv1.New(loggerv1.Config{ - Log: c.RootLog, + Log: c.RootLog, LaunchLevel: c.LaunchLogLevel, }), SVIDServer: svidv1.New(svidv1.Config{ From 10ac0f92a4f96067832ab73bfa2313db6522df94 Mon Sep 17 00:00:00 2001 From: Edwin Buck Date: Mon, 4 Mar 2024 20:58:37 -0600 Subject: [PATCH 12/20] Fixes for small code review points. Additions for testing under endpoints_test.go Signed-off-by: Edwin Buck --- cmd/spire-server/cli/logger/printers.go | 2 +- pkg/common/api/middleware/names.go | 2 + pkg/server/api/logger/v1/service.go | 2 +- pkg/server/endpoints/endpoints_test.go | 60 +++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 2 deletions(-) diff --git a/cmd/spire-server/cli/logger/printers.go b/cmd/spire-server/cli/logger/printers.go index c88a7c91cc..8562dab963 100644 --- a/cmd/spire-server/cli/logger/printers.go +++ b/cmd/spire-server/cli/logger/printers.go @@ -26,7 +26,7 @@ func PrettyPrintLogger(env *commoncli.Env, results ...any) error { logrusLaunch, found := serverlogger.LogrusLevel[apiLogger.LaunchLevel] if !found { - return errors.New("internal error: returned current log level is undefined; please report this as a bug") + return errors.New("internal error: returned launch log level is undefined; please report this as a bug") } launchText, err := logrusLaunch.MarshalText() if err != nil { diff --git a/pkg/common/api/middleware/names.go b/pkg/common/api/middleware/names.go index 3957403487..acacbbfa37 100644 --- a/pkg/common/api/middleware/names.go +++ b/pkg/common/api/middleware/names.go @@ -20,6 +20,7 @@ const ( HealthServiceName = "grpc.health.v1.Health" HealthServiceShortName = "Health" LoggerServiceName = "logger.v1.Logger" + LoggerServiceShortName = "Logger" DelegatedIdentityServiceName = "spire.api.agent.delegatedidentity.v1.DelegatedIdentity" DelegatedIdentityServiceShortName = "DelegatedIdentity" ServerReflectionServiceName = "grpc.reflection.v1.ServerReflection" @@ -34,6 +35,7 @@ var ( WorkloadAPIServiceName, WorkloadAPIServiceShortName, EnvoySDSv3ServiceName, EnvoySDSv3ServiceShortName, HealthServiceName, HealthServiceShortName, + LoggerServiceName, LoggerServiceShortName, DelegatedIdentityServiceName, DelegatedIdentityServiceShortName, ) diff --git a/pkg/server/api/logger/v1/service.go b/pkg/server/api/logger/v1/service.go index da35040b9f..db1237b201 100644 --- a/pkg/server/api/logger/v1/service.go +++ b/pkg/server/api/logger/v1/service.go @@ -33,7 +33,7 @@ type Service struct { func New(config Config) *Service { config.Log.WithFields(logrus.Fields{ "LaunchLevel": config.LaunchLevel, - }).Info("Logger service Configured") + }).Info("Logger service configured") return &Service{ log: config.Log, launchLevel: config.LaunchLevel, diff --git a/pkg/server/endpoints/endpoints_test.go b/pkg/server/endpoints/endpoints_test.go index 1e42828742..8cefb39081 100644 --- a/pkg/server/endpoints/endpoints_test.go +++ b/pkg/server/endpoints/endpoints_test.go @@ -19,6 +19,7 @@ import ( bundlev1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/bundle/v1" debugv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/debug/v1" entryv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/entry/v1" + loggerv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/logger/v1" svidv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/svid/v1" trustdomainv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/trustdomain/v1" "github.com/spiffe/spire-api-sdk/proto/spire/api/types" @@ -111,6 +112,7 @@ func TestNew(t *testing.T) { assert.NotNil(t, endpoints.APIServers.DebugServer) assert.NotNil(t, endpoints.APIServers.EntryServer) assert.NotNil(t, endpoints.APIServers.HealthServer) + assert.NotNil(t, endpoints.APIServers.LoggerServer) assert.NotNil(t, endpoints.APIServers.SVIDServer) assert.NotNil(t, endpoints.BundleEndpointServer) assert.NotNil(t, endpoints.EntryFetcherPruneEventsTask) @@ -209,6 +211,7 @@ func TestListenAndServe(t *testing.T) { DebugServer: debugServer{}, EntryServer: entryServer{}, HealthServer: healthServer{}, + LoggerServer: loggerServer{}, SVIDServer: svidServer{}, TrustDomainServer: trustDomainServer{}, }, @@ -302,6 +305,9 @@ func TestListenAndServe(t *testing.T) { t.Run("Health", func(t *testing.T) { testHealthAPI(ctx, t, conns) }) + t.Run("Logger", func(t *testing.T) { + testLoggerAPI(ctx, t, conns) + }) t.Run("Bundle", func(t *testing.T) { testBundleAPI(ctx, t, conns) }) @@ -519,6 +525,44 @@ func testHealthAPI(ctx context.Context, t *testing.T, conns testConns) { }) } +func testLoggerAPI(ctx context.Context, t *testing.T, conns testConns) { + t.Run("Local", func(t *testing.T) { + testAuthorization(ctx, t, loggerv1.NewLoggerClient(conns.local), map[string]bool{ + "GetLogger": true, + "SetLogLevel": true, + "ResetLogLevel": true, + }) + }) + + t.Run("NoAuth", func(t *testing.T) { + assertServiceUnavailable(ctx, t, loggerv1.NewLoggerClient(conns.noAuth)) + }) + + t.Run("Agent", func(t *testing.T) { + assertServiceUnavailable(ctx, t, loggerv1.NewLoggerClient(conns.agent)) + }) + + t.Run("Admin", func(t *testing.T) { + testAuthorization(ctx, t, loggerv1.NewLoggerClient(conns.admin), map[string]bool{ + "GetLogger": true, + "SetLogLevel": true, + "ResetLogLevel": true, + }) + }) + + t.Run("Federated Admin", func(t *testing.T) { + testAuthorization(ctx, t, loggerv1.NewLoggerClient(conns.federatedAdmin), map[string]bool{ + "GetLogger": true, + "SetLogLevel": true, + "ResetLogLevel": true, + }) + }) + + t.Run("Downstream", func(t *testing.T) { + assertServiceUnavailable(ctx, t, loggerv1.NewLoggerClient(conns.downstream)) + }) +} + func testDebugAPI(ctx context.Context, t *testing.T, conns testConns) { t.Run("Local", func(t *testing.T) { testAuthorization(ctx, t, debugv1.NewDebugClient(conns.local), map[string]bool{ @@ -1149,6 +1193,22 @@ func (healthServer) Watch(_ *grpc_health_v1.HealthCheckRequest, stream grpc_heal return stream.Send(&grpc_health_v1.HealthCheckResponse{}) } +type loggerServer struct { + loggerv1.UnsafeLoggerServer +} + +func (loggerServer) GetLogger(context.Context, *loggerv1.GetLoggerRequest) (*types.Logger, error) { + return &types.Logger{}, nil +} + +func (loggerServer) SetLogLevel(context.Context, *loggerv1.SetLogLevelRequest) (*types.Logger, error) { + return &types.Logger{}, nil +} + +func (loggerServer) ResetLogLevel(context.Context, *loggerv1.ResetLogLevelRequest) (*types.Logger, error) { + return &types.Logger{}, nil +} + type svidServer struct { svidv1.UnsafeSVIDServer } From 2c35b8bf687ac5c98e788dc486c59ea474012ac6 Mon Sep 17 00:00:00 2001 From: Edwin Buck Date: Mon, 4 Mar 2024 21:18:06 -0600 Subject: [PATCH 13/20] Flipped all testing of various access methods to true until issue 4940 is fixed. Removed allow_admin from policy_data.json until determination can be made to make this allow_admin Signed-off-by: Edwin Buck --- pkg/server/authpolicy/policy_data.json | 3 --- pkg/server/endpoints/endpoints_test.go | 12 ++---------- 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/pkg/server/authpolicy/policy_data.json b/pkg/server/authpolicy/policy_data.json index e14d134c72..d2363a1653 100644 --- a/pkg/server/authpolicy/policy_data.json +++ b/pkg/server/authpolicy/policy_data.json @@ -115,17 +115,14 @@ }, { "full_method": "/spire.api.server.logger.v1.Logger/GetLogger", - "allow_admin": true, "allow_local": true }, { "full_method": "/spire.api.server.logger.v1.Logger/SetLogLevel", - "allow_admin": true, "allow_local": true }, { "full_method": "/spire.api.server.logger.v1.Logger/ResetLogLevel", - "allow_admin": true, "allow_local": true }, { diff --git a/pkg/server/endpoints/endpoints_test.go b/pkg/server/endpoints/endpoints_test.go index 8cefb39081..542aafba4a 100644 --- a/pkg/server/endpoints/endpoints_test.go +++ b/pkg/server/endpoints/endpoints_test.go @@ -543,19 +543,11 @@ func testLoggerAPI(ctx context.Context, t *testing.T, conns testConns) { }) t.Run("Admin", func(t *testing.T) { - testAuthorization(ctx, t, loggerv1.NewLoggerClient(conns.admin), map[string]bool{ - "GetLogger": true, - "SetLogLevel": true, - "ResetLogLevel": true, - }) + assertServiceUnavailable(ctx, t, loggerv1.NewLoggerClient(conns.admin)) }) t.Run("Federated Admin", func(t *testing.T) { - testAuthorization(ctx, t, loggerv1.NewLoggerClient(conns.federatedAdmin), map[string]bool{ - "GetLogger": true, - "SetLogLevel": true, - "ResetLogLevel": true, - }) + assertServiceUnavailable(ctx, t, loggerv1.NewLoggerClient(conns.federatedAdmin)) }) t.Run("Downstream", func(t *testing.T) { From daeb76de8c44ca31c1ef96f4e53c6c227e65b7ee Mon Sep 17 00:00:00 2001 From: Edwin Buck Date: Tue, 5 Mar 2024 22:46:32 -0600 Subject: [PATCH 14/20] Add Logger GRPC Service unit testing. Signed-off-by: Edwin Buck --- pkg/server/api/logger/v1/levels_test.go | 107 +++ pkg/server/api/logger/v1/service.go | 10 +- pkg/server/api/logger/v1/service_test.go | 900 +++++++++++++++++++++++ 3 files changed, 1011 insertions(+), 6 deletions(-) create mode 100644 pkg/server/api/logger/v1/levels_test.go create mode 100644 pkg/server/api/logger/v1/service_test.go diff --git a/pkg/server/api/logger/v1/levels_test.go b/pkg/server/api/logger/v1/levels_test.go new file mode 100644 index 0000000000..9b40ec879a --- /dev/null +++ b/pkg/server/api/logger/v1/levels_test.go @@ -0,0 +1,107 @@ +package logger_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/sirupsen/logrus" + "github.com/spiffe/spire-api-sdk/proto/spire/api/types" + "github.com/spiffe/spire/pkg/server/api/logger/v1" +) + +func TestAPILevelValues(t *testing.T) { + for _, tt := range []struct { + name string + logrusLevel logrus.Level + expectedLevel types.LogLevel + }{ + { + name: "test logrus.PanicLevel fetches types.LogLevel_PANIC", + logrusLevel: logrus.PanicLevel, + expectedLevel: types.LogLevel_PANIC, + }, + { + name: "test logrus.FatalLevel fetches types.LogLevel_FATAL", + logrusLevel: logrus.FatalLevel, + expectedLevel: types.LogLevel_FATAL, + }, + { + name: "test logrus.ErrorLevel fetches types.LogLevel_ERROR", + logrusLevel: logrus.ErrorLevel, + expectedLevel: types.LogLevel_ERROR, + }, + { + name: "test logrus.WarnLevel fetches types.LogLevel_WARN", + logrusLevel: logrus.WarnLevel, + expectedLevel: types.LogLevel_WARN, + }, + { + name: "test logrus.InfoLevel fetches types.LogLevel_INFO", + logrusLevel: logrus.InfoLevel, + expectedLevel: types.LogLevel_INFO, + }, + { + name: "test logrus.DebugLevel fetches types.LogLevel_DEBUG", + logrusLevel: logrus.DebugLevel, + expectedLevel: types.LogLevel_DEBUG, + }, + { + name: "test logrus.TraceLevel fetches types.LogLevel_TRACE", + logrusLevel: logrus.TraceLevel, + expectedLevel: types.LogLevel_TRACE, + }, + } { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, logger.APILevel[tt.logrusLevel], tt.expectedLevel) + }) + } +} + +func TestLogrusLevelValues(t *testing.T) { + for _, tt := range []struct { + name string + apiLevel types.LogLevel + expectedLevel logrus.Level + }{ + { + name: "test types.LogLevel_PANIC fetches logrus.PanicLevel", + apiLevel: types.LogLevel_PANIC, + expectedLevel: logrus.PanicLevel, + }, + { + name: "test types.LogLevel_FATAL fetches logrus.FatalLevel", + apiLevel: types.LogLevel_FATAL, + expectedLevel: logrus.FatalLevel, + }, + { + name: "test types.LogLevel_ERROR fetches logrus.ErrorLevel", + apiLevel: types.LogLevel_ERROR, + expectedLevel: logrus.ErrorLevel, + }, + { + name: "test types.LogLevel_WARN fetches logrus.WarnLevel", + apiLevel: types.LogLevel_WARN, + expectedLevel: logrus.WarnLevel, + }, + { + name: "test types.LogLevel_INFO fetches logrus.InfoLevel", + apiLevel: types.LogLevel_INFO, + expectedLevel: logrus.InfoLevel, + }, + { + name: "test types.LogLevel_DEBUG fetches logrus.DebugLevel", + apiLevel: types.LogLevel_DEBUG, + expectedLevel: logrus.DebugLevel, + }, + { + name: "test types.LogLevel_TRACE fetches logrus.TraceLevel", + apiLevel: types.LogLevel_TRACE, + expectedLevel: logrus.TraceLevel, + }, + } { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, logger.LogrusLevel[tt.apiLevel], tt.expectedLevel) + }) + } +} diff --git a/pkg/server/api/logger/v1/service.go b/pkg/server/api/logger/v1/service.go index db1237b201..c18bdd7e61 100644 --- a/pkg/server/api/logger/v1/service.go +++ b/pkg/server/api/logger/v1/service.go @@ -3,7 +3,6 @@ package logger import ( "context" "fmt" - "strings" loggerv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/logger/v1" apitype "github.com/spiffe/spire-api-sdk/proto/spire/api/types" @@ -40,8 +39,8 @@ func New(config Config) *Service { } } -func RegisterService(server *grpc.Server, service *Service) { - loggerv1.RegisterLoggerServer(server, service) +func RegisterService(s grpc.ServiceRegistrar, service *Service) { + loggerv1.RegisterLoggerServer(s, service) } func (service *Service) GetLogger(_ context.Context, _ *loggerv1.GetLoggerRequest) (*apitype.Logger, error) { @@ -55,11 +54,10 @@ func (service *Service) GetLogger(_ context.Context, _ *loggerv1.GetLoggerReques func (service *Service) SetLogLevel(_ context.Context, req *loggerv1.SetLogLevelRequest) (*apitype.Logger, error) { if req.NewLevel == apitype.LogLevel_UNSPECIFIED { - return nil, fmt.Errorf("Invalid request NewLevel value cannot be LogLevel_UNSPECIFIED") + return nil, fmt.Errorf("Invalid request, NewLevel value cannot be LogLevel_UNSPECIFIED") } service.log.WithFields(logrus.Fields{ - "ApiLogLevel": strings.ToLower(apitype.LogLevel_name[int32(req.NewLevel)]), - "LogrusLevel": LogrusLevel[req.NewLevel].String(), + "NewLevel": LogrusLevel[req.NewLevel].String(), }).Info("SetLogLevel Called") service.log.SetLevel(LogrusLevel[req.NewLevel]) logger := &apitype.Logger{ diff --git a/pkg/server/api/logger/v1/service_test.go b/pkg/server/api/logger/v1/service_test.go new file mode 100644 index 0000000000..31a8ddba5d --- /dev/null +++ b/pkg/server/api/logger/v1/service_test.go @@ -0,0 +1,900 @@ +package logger_test + +import ( + "context" + "testing" + + "github.com/sirupsen/logrus/hooks/test" + "github.com/spiffe/spire/test/grpctest" + "github.com/spiffe/spire/test/spiretest" + "github.com/stretchr/testify/require" + + "github.com/sirupsen/logrus" + loggerv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/logger/v1" + apitype "github.com/spiffe/spire-api-sdk/proto/spire/api/types" + "github.com/spiffe/spire/pkg/server/api/logger/v1" + "github.com/spiffe/spire/pkg/server/api/rpccontext" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" +) + +func TestGetLogger(t *testing.T) { + for _, tt := range []struct { + name string + launchLevel logrus.Level + + expectedErr error + expectedResponse *apitype.Logger + expectedLogs []spiretest.LogEntry + }{ + { + name: "test GetLogger on initialized to PANIC", + launchLevel: logrus.PanicLevel, + + expectedResponse: &apitype.Logger{ + CurrentLevel: apitype.LogLevel_PANIC, + LaunchLevel: apitype.LogLevel_PANIC, + }, + // no outputted log messages, as the are at INFO level + expectedLogs: nil, + }, + { + name: "test GetLogger on initialized to FATAL", + launchLevel: logrus.FatalLevel, + + expectedResponse: &apitype.Logger{ + CurrentLevel: apitype.LogLevel_FATAL, + LaunchLevel: apitype.LogLevel_FATAL, + }, + // no outputted log messages, as the are at INFO level + expectedLogs: nil, + }, + { + name: "test GetLogger on initialized to ERROR", + launchLevel: logrus.ErrorLevel, + + expectedResponse: &apitype.Logger{ + CurrentLevel: apitype.LogLevel_ERROR, + LaunchLevel: apitype.LogLevel_ERROR, + }, + // no outputted log messages, as the are at INFO level + expectedLogs: nil, + }, + { + name: "test GetLogger on initialized to WARN", + launchLevel: logrus.WarnLevel, + + expectedResponse: &apitype.Logger{ + CurrentLevel: apitype.LogLevel_WARN, + LaunchLevel: apitype.LogLevel_WARN, + }, + // no outputted log messages, as the are at INFO level + expectedLogs: nil, + }, + { + name: "test GetLogger on initialized to INFO", + launchLevel: logrus.InfoLevel, + + expectedResponse: &apitype.Logger{ + CurrentLevel: apitype.LogLevel_INFO, + LaunchLevel: apitype.LogLevel_INFO, + }, + expectedLogs: []spiretest.LogEntry{ + { + Level: logrus.InfoLevel, + Message: "Logger service configured", + Data: logrus.Fields{ + "LaunchLevel": "info", + }, + }, + { + Level: logrus.InfoLevel, + Message: "GetLogger Called", + }, + }, + }, + { + name: "test GetLogger on initialized to DEBUG", + launchLevel: logrus.DebugLevel, + + expectedResponse: &apitype.Logger{ + CurrentLevel: apitype.LogLevel_DEBUG, + LaunchLevel: apitype.LogLevel_DEBUG, + }, + expectedLogs: []spiretest.LogEntry{ + { + Level: logrus.InfoLevel, + Message: "Logger service configured", + Data: logrus.Fields{ + "LaunchLevel": "debug", + }, + }, + { + Level: logrus.InfoLevel, + Message: "GetLogger Called", + }, + }, + }, + { + name: "test GetLogger on initialized to TRACE", + launchLevel: logrus.TraceLevel, + + expectedResponse: &apitype.Logger{ + CurrentLevel: apitype.LogLevel_TRACE, + LaunchLevel: apitype.LogLevel_TRACE, + }, + expectedLogs: []spiretest.LogEntry{ + { + Level: logrus.InfoLevel, + Message: "Logger service configured", + Data: logrus.Fields{ + "LaunchLevel": "trace", + }, + }, + { + Level: logrus.InfoLevel, + Message: "GetLogger Called", + }, + }, + }, + } { + tt := tt + t.Run(tt.name, func(t *testing.T) { + test := setupServiceTest(t, tt.launchLevel) + defer test.Cleanup() + + resp, err := test.client.GetLogger(context.Background(), &loggerv1.GetLoggerRequest{}) + require.Equal(t, err, tt.expectedErr) + spiretest.AssertLogs(t, test.logHook.AllEntries(), tt.expectedLogs) + spiretest.RequireProtoEqual(t, resp, tt.expectedResponse) + }) + } +} + +// After changing the log level, gets the logger to check the log impact +func TestSetLoggerThenGetLogger(t *testing.T) { + for _, tt := range []struct { + name string + launchLevel logrus.Level + setLogLevelRequest *loggerv1.SetLogLevelRequest + + expectedErr error + expectedResponse *apitype.Logger + expectedLogs []spiretest.LogEntry + }{ + { + name: "test SetLogger to FATAL on initialized to PANIC", + launchLevel: logrus.PanicLevel, + setLogLevelRequest: &loggerv1.SetLogLevelRequest{ + NewLevel: apitype.LogLevel_FATAL, + }, + + expectedResponse: &apitype.Logger{ + CurrentLevel: apitype.LogLevel_FATAL, + LaunchLevel: apitype.LogLevel_PANIC, + }, + expectedLogs: nil, + }, + { + name: "test SetLogger to INFO on initialized to PANIC", + launchLevel: logrus.PanicLevel, + setLogLevelRequest: &loggerv1.SetLogLevelRequest{ + NewLevel: apitype.LogLevel_INFO, + }, + + expectedResponse: &apitype.Logger{ + CurrentLevel: apitype.LogLevel_INFO, + LaunchLevel: apitype.LogLevel_PANIC, + }, + // only the ending get logger will log + expectedLogs: []spiretest.LogEntry{ + { + Level: logrus.InfoLevel, + Message: "GetLogger Called", + }, + }, + }, + { + name: "test SetLogger to DEBUG on initialized to PANIC", + launchLevel: logrus.PanicLevel, + setLogLevelRequest: &loggerv1.SetLogLevelRequest{ + NewLevel: apitype.LogLevel_DEBUG, + }, + + expectedResponse: &apitype.Logger{ + CurrentLevel: apitype.LogLevel_DEBUG, + LaunchLevel: apitype.LogLevel_PANIC, + }, + // only the ending get logger will log + expectedLogs: []spiretest.LogEntry{ + { + Level: logrus.InfoLevel, + Message: "GetLogger Called", + }, + }, + }, + { + name: "test SetLogger to PANIC on initialized to INFO", + launchLevel: logrus.InfoLevel, + setLogLevelRequest: &loggerv1.SetLogLevelRequest{ + NewLevel: apitype.LogLevel_PANIC, + }, + + expectedResponse: &apitype.Logger{ + CurrentLevel: apitype.LogLevel_PANIC, + LaunchLevel: apitype.LogLevel_INFO, + }, + // the ending getlogger will be suppressed + expectedLogs: []spiretest.LogEntry{ + { + Level: logrus.InfoLevel, + Message: "Logger service configured", + Data: logrus.Fields{ + "LaunchLevel": "info", + }, + }, + { + Level: logrus.InfoLevel, + Message: "SetLogLevel Called", + Data: logrus.Fields{ + "NewLevel": "panic", + }, + }, + }, + }, + { + name: "test SetLogger to INFO on initialized to INFO", + launchLevel: logrus.InfoLevel, + setLogLevelRequest: &loggerv1.SetLogLevelRequest{ + NewLevel: apitype.LogLevel_INFO, + }, + + expectedResponse: &apitype.Logger{ + CurrentLevel: apitype.LogLevel_INFO, + LaunchLevel: apitype.LogLevel_INFO, + }, + expectedLogs: []spiretest.LogEntry{ + { + Level: logrus.InfoLevel, + Message: "Logger service configured", + Data: logrus.Fields{ + "LaunchLevel": "info", + }, + }, + { + Level: logrus.InfoLevel, + Message: "SetLogLevel Called", + Data: logrus.Fields{ + "NewLevel": "info", + }, + }, + { + Level: logrus.InfoLevel, + Message: "GetLogger Called", + }, + }, + }, + { + name: "test SetLogger to DEBUG on initialized to INFO", + launchLevel: logrus.InfoLevel, + setLogLevelRequest: &loggerv1.SetLogLevelRequest{ + NewLevel: apitype.LogLevel_DEBUG, + }, + + expectedResponse: &apitype.Logger{ + CurrentLevel: apitype.LogLevel_DEBUG, + LaunchLevel: apitype.LogLevel_INFO, + }, + expectedLogs: []spiretest.LogEntry{ + { + Level: logrus.InfoLevel, + Message: "Logger service configured", + Data: logrus.Fields{ + "LaunchLevel": "info", + }, + }, + { + Level: logrus.InfoLevel, + Message: "SetLogLevel Called", + Data: logrus.Fields{ + "NewLevel": "debug", + }, + }, + { + Level: logrus.InfoLevel, + Message: "GetLogger Called", + }, + }, + }, + { + name: "test SetLogger to PANIC on initialized to TRACE", + launchLevel: logrus.TraceLevel, + setLogLevelRequest: &loggerv1.SetLogLevelRequest{ + NewLevel: apitype.LogLevel_PANIC, + }, + + expectedResponse: &apitype.Logger{ + CurrentLevel: apitype.LogLevel_PANIC, + LaunchLevel: apitype.LogLevel_TRACE, + }, + // the ending getlogger will be suppressed + expectedLogs: []spiretest.LogEntry{ + { + Level: logrus.InfoLevel, + Message: "Logger service configured", + Data: logrus.Fields{ + "LaunchLevel": "trace", + }, + }, + { + Level: logrus.InfoLevel, + Message: "SetLogLevel Called", + Data: logrus.Fields{ + "NewLevel": "panic", + }, + }, + }, + }, + { + name: "test SetLogger to INFO on initialized to TRACE", + launchLevel: logrus.TraceLevel, + setLogLevelRequest: &loggerv1.SetLogLevelRequest{ + NewLevel: apitype.LogLevel_INFO, + }, + + expectedResponse: &apitype.Logger{ + CurrentLevel: apitype.LogLevel_INFO, + LaunchLevel: apitype.LogLevel_TRACE, + }, + expectedLogs: []spiretest.LogEntry{ + { + Level: logrus.InfoLevel, + Message: "Logger service configured", + Data: logrus.Fields{ + "LaunchLevel": "trace", + }, + }, + { + Level: logrus.InfoLevel, + Message: "SetLogLevel Called", + Data: logrus.Fields{ + "NewLevel": "info", + }, + }, + { + Level: logrus.InfoLevel, + Message: "GetLogger Called", + }, + }, + }, + { + name: "test SetLogger to DEBUG on initialized to TRACE", + launchLevel: logrus.TraceLevel, + setLogLevelRequest: &loggerv1.SetLogLevelRequest{ + NewLevel: apitype.LogLevel_DEBUG, + }, + + expectedResponse: &apitype.Logger{ + CurrentLevel: apitype.LogLevel_DEBUG, + LaunchLevel: apitype.LogLevel_TRACE, + }, + expectedLogs: []spiretest.LogEntry{ + { + Level: logrus.InfoLevel, + Message: "Logger service configured", + Data: logrus.Fields{ + "LaunchLevel": "trace", + }, + }, + { + Level: logrus.InfoLevel, + Message: "SetLogLevel Called", + Data: logrus.Fields{ + "NewLevel": "debug", + }, + }, + { + Level: logrus.InfoLevel, + Message: "GetLogger Called", + }, + }, + }, + } { + tt := tt + t.Run(tt.name, func(t *testing.T) { + test := setupServiceTest(t, tt.launchLevel) + defer test.Cleanup() + + resp, err := test.client.SetLogLevel(context.Background(), tt.setLogLevelRequest) + require.Equal(t, err, tt.expectedErr) + spiretest.RequireProtoEqual(t, resp, tt.expectedResponse) + resp, err = test.client.GetLogger(context.Background(), &loggerv1.GetLoggerRequest{}) + spiretest.RequireProtoEqual(t, resp, tt.expectedResponse) + + spiretest.AssertLogs(t, test.logHook.AllEntries(), tt.expectedLogs) + }) + } +} + +// After changing the log level, gets the logger to check the log impact +// After resetting the log level, gets the logger to check the log impact +func TestResetLogger(t *testing.T) { + for _, tt := range []struct { + name string + launchLevel logrus.Level + setLogLevelRequest *loggerv1.SetLogLevelRequest + + expectedErr error + expectedResponse *apitype.Logger + expectedLogs []spiretest.LogEntry + }{ + { + name: "test PANIC Logger set to FATAL then RESET", + launchLevel: logrus.PanicLevel, + setLogLevelRequest: &loggerv1.SetLogLevelRequest{ + NewLevel: apitype.LogLevel_FATAL, + }, + + expectedResponse: &apitype.Logger{ + CurrentLevel: apitype.LogLevel_PANIC, + LaunchLevel: apitype.LogLevel_PANIC, + }, + expectedLogs: nil, + }, + { + name: "test PANIC Logger set to INFO then RESET", + launchLevel: logrus.PanicLevel, + setLogLevelRequest: &loggerv1.SetLogLevelRequest{ + NewLevel: apitype.LogLevel_INFO, + }, + + expectedResponse: &apitype.Logger{ + CurrentLevel: apitype.LogLevel_PANIC, + LaunchLevel: apitype.LogLevel_PANIC, + }, + // only the ending get logger will log + expectedLogs: []spiretest.LogEntry{ + { + Level: logrus.InfoLevel, + Message: "GetLogger Called", + }, + { + Level: logrus.InfoLevel, + Message: "ResetLogLevel Called", + }, + }, + }, + { + name: "test PANIC Logger set to DEBUG then RESET", + launchLevel: logrus.PanicLevel, + setLogLevelRequest: &loggerv1.SetLogLevelRequest{ + NewLevel: apitype.LogLevel_DEBUG, + }, + + expectedResponse: &apitype.Logger{ + CurrentLevel: apitype.LogLevel_PANIC, + LaunchLevel: apitype.LogLevel_PANIC, + }, + // only the ending get logger will log + expectedLogs: []spiretest.LogEntry{ + { + Level: logrus.InfoLevel, + Message: "GetLogger Called", + }, + { + Level: logrus.InfoLevel, + Message: "ResetLogLevel Called", + }, + }, + }, + { + name: "test INFO Logger set to PANIC and then RESET", + launchLevel: logrus.InfoLevel, + setLogLevelRequest: &loggerv1.SetLogLevelRequest{ + NewLevel: apitype.LogLevel_PANIC, + }, + + expectedResponse: &apitype.Logger{ + CurrentLevel: apitype.LogLevel_INFO, + LaunchLevel: apitype.LogLevel_INFO, + }, + // the ending getlogger will be suppressed + expectedLogs: []spiretest.LogEntry{ + { + Level: logrus.InfoLevel, + Message: "Logger service configured", + Data: logrus.Fields{ + "LaunchLevel": "info", + }, + }, + { + Level: logrus.InfoLevel, + Message: "SetLogLevel Called", + Data: logrus.Fields{ + "NewLevel": "panic", + }, + }, + // the second get, after the reset + { + Level: logrus.InfoLevel, + Message: "GetLogger Called", + }, + }, + }, + { + name: "test INFO Logger set to INFO and then RESET", + launchLevel: logrus.InfoLevel, + setLogLevelRequest: &loggerv1.SetLogLevelRequest{ + NewLevel: apitype.LogLevel_INFO, + }, + + expectedResponse: &apitype.Logger{ + CurrentLevel: apitype.LogLevel_INFO, + LaunchLevel: apitype.LogLevel_INFO, + }, + expectedLogs: []spiretest.LogEntry{ + { + Level: logrus.InfoLevel, + Message: "Logger service configured", + Data: logrus.Fields{ + "LaunchLevel": "info", + }, + }, + { + Level: logrus.InfoLevel, + Message: "SetLogLevel Called", + Data: logrus.Fields{ + "NewLevel": "info", + }, + }, + { + Level: logrus.InfoLevel, + Message: "GetLogger Called", + }, + { + Level: logrus.InfoLevel, + Message: "ResetLogLevel Called", + }, + { + Level: logrus.InfoLevel, + Message: "GetLogger Called", + }, + }, + }, + { + name: "test INFO Logger set to DEBUG and then RESET", + launchLevel: logrus.InfoLevel, + setLogLevelRequest: &loggerv1.SetLogLevelRequest{ + NewLevel: apitype.LogLevel_DEBUG, + }, + + expectedResponse: &apitype.Logger{ + CurrentLevel: apitype.LogLevel_INFO, + LaunchLevel: apitype.LogLevel_INFO, + }, + expectedLogs: []spiretest.LogEntry{ + { + Level: logrus.InfoLevel, + Message: "Logger service configured", + Data: logrus.Fields{ + "LaunchLevel": "info", + }, + }, + { + Level: logrus.InfoLevel, + Message: "SetLogLevel Called", + Data: logrus.Fields{ + "NewLevel": "debug", + }, + }, + { + Level: logrus.InfoLevel, + Message: "GetLogger Called", + }, + { + Level: logrus.InfoLevel, + Message: "ResetLogLevel Called", + }, + { + Level: logrus.InfoLevel, + Message: "GetLogger Called", + }, + }, + }, + { + name: "test TRACE Logger set to PANIC and then RESET", + launchLevel: logrus.TraceLevel, + setLogLevelRequest: &loggerv1.SetLogLevelRequest{ + NewLevel: apitype.LogLevel_PANIC, + }, + + expectedResponse: &apitype.Logger{ + CurrentLevel: apitype.LogLevel_TRACE, + LaunchLevel: apitype.LogLevel_TRACE, + }, + // the ending getlogger will be suppressed + expectedLogs: []spiretest.LogEntry{ + { + Level: logrus.InfoLevel, + Message: "Logger service configured", + Data: logrus.Fields{ + "LaunchLevel": "trace", + }, + }, + { + Level: logrus.InfoLevel, + Message: "SetLogLevel Called", + Data: logrus.Fields{ + "NewLevel": "panic", + }, + }, + // the second get logger, after the reset + { + Level: logrus.InfoLevel, + Message: "GetLogger Called", + }, + }, + }, + { + name: "test TRACE Logger set to INFO and then RESET", + launchLevel: logrus.TraceLevel, + setLogLevelRequest: &loggerv1.SetLogLevelRequest{ + NewLevel: apitype.LogLevel_INFO, + }, + + expectedResponse: &apitype.Logger{ + CurrentLevel: apitype.LogLevel_TRACE, + LaunchLevel: apitype.LogLevel_TRACE, + }, + expectedLogs: []spiretest.LogEntry{ + { + Level: logrus.InfoLevel, + Message: "Logger service configured", + Data: logrus.Fields{ + "LaunchLevel": "trace", + }, + }, + { + Level: logrus.InfoLevel, + Message: "SetLogLevel Called", + Data: logrus.Fields{ + "NewLevel": "info", + }, + }, + { + Level: logrus.InfoLevel, + Message: "GetLogger Called", + }, + { + Level: logrus.InfoLevel, + Message: "ResetLogLevel Called", + }, + { + Level: logrus.InfoLevel, + Message: "GetLogger Called", + }, + }, + }, + { + name: "test TRACE Logger set to DEBUG and then RESET", + launchLevel: logrus.TraceLevel, + setLogLevelRequest: &loggerv1.SetLogLevelRequest{ + NewLevel: apitype.LogLevel_DEBUG, + }, + + expectedResponse: &apitype.Logger{ + CurrentLevel: apitype.LogLevel_TRACE, + LaunchLevel: apitype.LogLevel_TRACE, + }, + expectedLogs: []spiretest.LogEntry{ + { + Level: logrus.InfoLevel, + Message: "Logger service configured", + Data: logrus.Fields{ + "LaunchLevel": "trace", + }, + }, + { + Level: logrus.InfoLevel, + Message: "SetLogLevel Called", + Data: logrus.Fields{ + "NewLevel": "debug", + }, + }, + { + Level: logrus.InfoLevel, + Message: "GetLogger Called", + }, + { + Level: logrus.InfoLevel, + Message: "ResetLogLevel Called", + }, + { + Level: logrus.InfoLevel, + Message: "GetLogger Called", + }, + }, + }, + } { + tt := tt + t.Run(tt.name, func(t *testing.T) { + test := setupServiceTest(t, tt.launchLevel) + defer test.Cleanup() + + _, err := test.client.SetLogLevel(context.Background(), tt.setLogLevelRequest) + _, err = test.client.GetLogger(context.Background(), &loggerv1.GetLoggerRequest{}) + resp, err := test.client.ResetLogLevel(context.Background(), &loggerv1.ResetLogLevelRequest{}) + + require.Equal(t, err, tt.expectedErr) + spiretest.RequireProtoEqual(t, resp, tt.expectedResponse) + _, err = test.client.GetLogger(context.Background(), &loggerv1.GetLoggerRequest{}) + spiretest.AssertLogs(t, test.logHook.AllEntries(), tt.expectedLogs) + }) + } +} + +func TestUnsetSetLogLevelRequest(t *testing.T) { + for _, tt := range []struct { + name string + launchLevel logrus.Level + setLogLevelRequest *loggerv1.SetLogLevelRequest + + code codes.Code + expectedErr string + expectedResponse *apitype.Logger + expectedLogs []spiretest.LogEntry + }{ + { + name: "test PANIC Logger set without a log level", + launchLevel: logrus.PanicLevel, + setLogLevelRequest: &loggerv1.SetLogLevelRequest{}, + + code: codes.Unknown, + expectedErr: "Invalid request, NewLevel value cannot be LogLevel_UNSPECIFIED", + expectedResponse: nil, + // the error seems to clear the log capture + expectedLogs: nil, + }, + { + name: "test PANIC Logger set to UNSPECIFIED", + launchLevel: logrus.PanicLevel, + setLogLevelRequest: &loggerv1.SetLogLevelRequest{ + NewLevel: apitype.LogLevel_UNSPECIFIED, + }, + + code: codes.Unknown, + expectedErr: "Invalid request, NewLevel value cannot be LogLevel_UNSPECIFIED", + expectedResponse: nil, + // the error seems to clear the log capture + expectedLogs: nil, + }, + { + name: "test INFO Logger set without a log level", + launchLevel: logrus.InfoLevel, + setLogLevelRequest: &loggerv1.SetLogLevelRequest{}, + + code: codes.Unknown, + expectedErr: "Invalid request, NewLevel value cannot be LogLevel_UNSPECIFIED", + expectedResponse: nil, + expectedLogs: []spiretest.LogEntry{ + { + Level: logrus.InfoLevel, + Message: "Logger service configured", + Data: logrus.Fields{ + "LaunchLevel": "info", + }, + }, + }, + }, + { + name: "test INFO Logger set to UNSPECIFIED", + launchLevel: logrus.InfoLevel, + setLogLevelRequest: &loggerv1.SetLogLevelRequest{ + NewLevel: apitype.LogLevel_UNSPECIFIED, + }, + + code: codes.Unknown, + expectedErr: "Invalid request, NewLevel value cannot be LogLevel_UNSPECIFIED", + expectedResponse: nil, + expectedLogs: []spiretest.LogEntry{ + { + Level: logrus.InfoLevel, + Message: "Logger service configured", + Data: logrus.Fields{ + "LaunchLevel": "info", + }, + }, + }, + }, + { + name: "test DEBUG Logger set without a log level", + launchLevel: logrus.DebugLevel, + setLogLevelRequest: &loggerv1.SetLogLevelRequest{}, + + code: codes.Unknown, + expectedErr: "Invalid request, NewLevel value cannot be LogLevel_UNSPECIFIED", + expectedResponse: nil, + expectedLogs: []spiretest.LogEntry{ + { + Level: logrus.InfoLevel, + Message: "Logger service configured", + Data: logrus.Fields{ + "LaunchLevel": "debug", + }, + }, + }, + }, + { + name: "test DEBUG Logger set to UNSPECIFIED", + launchLevel: logrus.DebugLevel, + setLogLevelRequest: &loggerv1.SetLogLevelRequest{ + NewLevel: apitype.LogLevel_UNSPECIFIED, + }, + + code: codes.Unknown, + expectedErr: "Invalid request, NewLevel value cannot be LogLevel_UNSPECIFIED", + expectedResponse: nil, + expectedLogs: []spiretest.LogEntry{ + { + Level: logrus.InfoLevel, + Message: "Logger service configured", + Data: logrus.Fields{ + "LaunchLevel": "debug", + }, + }, + }, + }, + } { + tt := tt + t.Run(tt.name, func(t *testing.T) { + test := setupServiceTest(t, tt.launchLevel) + defer test.Cleanup() + + resp, err := test.client.SetLogLevel(context.Background(), tt.setLogLevelRequest) + spiretest.RequireGRPCStatusContains(t, err, tt.code, tt.expectedErr) + require.Nil(t, resp) + + spiretest.RequireProtoEqual(t, resp, tt.expectedResponse) + spiretest.AssertLogs(t, test.logHook.AllEntries(), tt.expectedLogs) + }) + } +} + +type serviceTest struct { + client loggerv1.LoggerClient + done func() + + logHook *test.Hook +} + +func (s *serviceTest) Cleanup() { + s.done() +} + +func setupServiceTest(t *testing.T, launchLevel logrus.Level) *serviceTest { + log, logHook := test.NewNullLogger() + // logger level should initially match the launch level + log.SetLevel(launchLevel) + service := logger.New(logger.Config{ + Log: log, + LaunchLevel: launchLevel, + }) + + registerFn := func(s grpc.ServiceRegistrar) { + logger.RegisterService(s, service) + } + overrideContext := func(ctx context.Context) context.Context { + ctx = rpccontext.WithLogger(ctx, log) + return ctx + } + server := grpctest.StartServer(t, registerFn, grpctest.OverrideContext(overrideContext)) + conn := server.Dial(t) + + test := &serviceTest{ + done: server.Stop, + logHook: logHook, + client: loggerv1.NewLoggerClient(conn), + } + + return test +} From b5c6058d6811b6d77b6d7301c4b4fb8860056196 Mon Sep 17 00:00:00 2001 From: Edwin Buck Date: Wed, 6 Mar 2024 00:03:51 -0600 Subject: [PATCH 15/20] Fix lint issues. Signed-off-by: Edwin Buck --- pkg/server/api/logger/v1/service_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/server/api/logger/v1/service_test.go b/pkg/server/api/logger/v1/service_test.go index 31a8ddba5d..35e75d29fc 100644 --- a/pkg/server/api/logger/v1/service_test.go +++ b/pkg/server/api/logger/v1/service_test.go @@ -405,10 +405,10 @@ func TestSetLoggerThenGetLogger(t *testing.T) { test := setupServiceTest(t, tt.launchLevel) defer test.Cleanup() - resp, err := test.client.SetLogLevel(context.Background(), tt.setLogLevelRequest) - require.Equal(t, err, tt.expectedErr) + resp, _ := test.client.SetLogLevel(context.Background(), tt.setLogLevelRequest) spiretest.RequireProtoEqual(t, resp, tt.expectedResponse) - resp, err = test.client.GetLogger(context.Background(), &loggerv1.GetLoggerRequest{}) + resp, err := test.client.GetLogger(context.Background(), &loggerv1.GetLoggerRequest{}) + require.Equal(t, err, tt.expectedErr) spiretest.RequireProtoEqual(t, resp, tt.expectedResponse) spiretest.AssertLogs(t, test.logHook.AllEntries(), tt.expectedLogs) @@ -721,13 +721,13 @@ func TestResetLogger(t *testing.T) { test := setupServiceTest(t, tt.launchLevel) defer test.Cleanup() - _, err := test.client.SetLogLevel(context.Background(), tt.setLogLevelRequest) - _, err = test.client.GetLogger(context.Background(), &loggerv1.GetLoggerRequest{}) + _, _ = test.client.SetLogLevel(context.Background(), tt.setLogLevelRequest) + _, _ = test.client.GetLogger(context.Background(), &loggerv1.GetLoggerRequest{}) resp, err := test.client.ResetLogLevel(context.Background(), &loggerv1.ResetLogLevelRequest{}) require.Equal(t, err, tt.expectedErr) spiretest.RequireProtoEqual(t, resp, tt.expectedResponse) - _, err = test.client.GetLogger(context.Background(), &loggerv1.GetLoggerRequest{}) + _, _ = test.client.GetLogger(context.Background(), &loggerv1.GetLoggerRequest{}) spiretest.AssertLogs(t, test.logHook.AllEntries(), tt.expectedLogs) }) } From 99661972e3a61a48412f84e5454d4b378db3ce9a Mon Sep 17 00:00:00 2001 From: Edwin Buck Date: Wed, 6 Mar 2024 00:20:54 -0600 Subject: [PATCH 16/20] Fix unit test that was unnoticed as the test was cached. Signed-off-by: Edwin Buck --- cmd/spire-server/cli/logger/get_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/spire-server/cli/logger/get_test.go b/cmd/spire-server/cli/logger/get_test.go index 002df8602b..484ba3ce3e 100644 --- a/cmd/spire-server/cli/logger/get_test.go +++ b/cmd/spire-server/cli/logger/get_test.go @@ -172,7 +172,7 @@ Launch Level : panic returnLogger: nil, }, expectReturnCode: 1, - expectStderr: `Error: error fetching logger: rpc error: code = Internal desc = grpc: error while marshaling: proto: Marshal called with nil + expectStderr: `Error: internal error: returned current log level is undefined; please report this as a bug `, }, } { From d07317e92b0fb817a2f24dbe2669de7c58cab7be Mon Sep 17 00:00:00 2001 From: Edwin Buck Date: Wed, 6 Mar 2024 01:04:22 -0600 Subject: [PATCH 17/20] Fix unit tests on windows. Signed-off-by: Edwin Buck --- cmd/spire-server/cli/logger/get_posix_test.go | 13 +++++++++++++ cmd/spire-server/cli/logger/get_test.go | 9 --------- cmd/spire-server/cli/logger/get_windows_test.go | 13 +++++++++++++ 3 files changed, 26 insertions(+), 9 deletions(-) create mode 100644 cmd/spire-server/cli/logger/get_posix_test.go create mode 100644 cmd/spire-server/cli/logger/get_windows_test.go diff --git a/cmd/spire-server/cli/logger/get_posix_test.go b/cmd/spire-server/cli/logger/get_posix_test.go new file mode 100644 index 0000000000..65542d8fef --- /dev/null +++ b/cmd/spire-server/cli/logger/get_posix_test.go @@ -0,0 +1,13 @@ +//go:build !windows + +package logger_test + +var ( + getUsage = `Usage of logger get: + -output value + Desired output format (pretty, json); default: pretty. + -socketPath string + Path to the SPIRE Server API socket (default "/tmp/spire-server/private/api.sock") +` +) + diff --git a/cmd/spire-server/cli/logger/get_test.go b/cmd/spire-server/cli/logger/get_test.go index 484ba3ce3e..b571f785b7 100644 --- a/cmd/spire-server/cli/logger/get_test.go +++ b/cmd/spire-server/cli/logger/get_test.go @@ -10,15 +10,6 @@ import ( "github.com/spiffe/spire/cmd/spire-server/cli/logger" ) -var ( - getUsage = `Usage of logger get: - -output value - Desired output format (pretty, json); default: pretty. - -socketPath string - Path to the SPIRE Server API socket (default "/tmp/spire-server/private/api.sock") -` -) - func TestGetHelp(t *testing.T) { test := setupCliTest(t, nil, logger.NewGetCommandWithEnv) test.client.Help() diff --git a/cmd/spire-server/cli/logger/get_windows_test.go b/cmd/spire-server/cli/logger/get_windows_test.go new file mode 100644 index 0000000000..9e20a07e9a --- /dev/null +++ b/cmd/spire-server/cli/logger/get_windows_test.go @@ -0,0 +1,13 @@ +//go:build windows + +package logger_test + +var ( + getUsage = `Usage of logger get: + -namedPipeName string + Pipe name of the SPIRE Server API named pipe (default "\\spire-server\\private\\api") + -output value + Desired output format (pretty, json); default: pretty. +` +) + From e4d675f3f3c486e02a0753702681eca1c21b2365 Mon Sep 17 00:00:00 2001 From: Edwin Buck Date: Wed, 6 Mar 2024 01:20:45 -0600 Subject: [PATCH 18/20] goimport get_posix_test.go Signed-off-by: Edwin Buck --- cmd/spire-server/cli/logger/get_posix_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/spire-server/cli/logger/get_posix_test.go b/cmd/spire-server/cli/logger/get_posix_test.go index 65542d8fef..9e5cf4b3db 100644 --- a/cmd/spire-server/cli/logger/get_posix_test.go +++ b/cmd/spire-server/cli/logger/get_posix_test.go @@ -10,4 +10,3 @@ var ( Path to the SPIRE Server API socket (default "/tmp/spire-server/private/api.sock") ` ) - From d8ea93dbab0644c62751b8f893d89d125240eee2 Mon Sep 17 00:00:00 2001 From: Edwin Buck Date: Wed, 6 Mar 2024 02:04:11 -0600 Subject: [PATCH 19/20] goimport format the windows test data in get_windows_test.go Signed-off-by: Edwin Buck --- cmd/spire-server/cli/logger/get_windows_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/spire-server/cli/logger/get_windows_test.go b/cmd/spire-server/cli/logger/get_windows_test.go index 9e20a07e9a..d7a1c53582 100644 --- a/cmd/spire-server/cli/logger/get_windows_test.go +++ b/cmd/spire-server/cli/logger/get_windows_test.go @@ -10,4 +10,3 @@ var ( Desired output format (pretty, json); default: pretty. ` ) - From 1c5c1441200d28018a6f2f3d9c0b31b7cc1fe8d5 Mon Sep 17 00:00:00 2001 From: Edwin Buck Date: Wed, 6 Mar 2024 02:18:40 -0600 Subject: [PATCH 20/20] Fix windows unit testing of logger set cli command. Signed-off-by: Edwin Buck --- cmd/spire-server/cli/logger/set_posix_test.go | 14 ++++++++++++++ cmd/spire-server/cli/logger/set_test.go | 11 ----------- cmd/spire-server/cli/logger/set_windows_test.go | 14 ++++++++++++++ 3 files changed, 28 insertions(+), 11 deletions(-) create mode 100644 cmd/spire-server/cli/logger/set_posix_test.go create mode 100644 cmd/spire-server/cli/logger/set_windows_test.go diff --git a/cmd/spire-server/cli/logger/set_posix_test.go b/cmd/spire-server/cli/logger/set_posix_test.go new file mode 100644 index 0000000000..1b84c77345 --- /dev/null +++ b/cmd/spire-server/cli/logger/set_posix_test.go @@ -0,0 +1,14 @@ +//go:build !windows + +package logger_test + +var ( + setUsage = `Usage of logger set: + -level string + The new log level, one of (panic, fatal, error, warn, info, debug, trace, launch) + -output value + Desired output format (pretty, json); default: pretty. + -socketPath string + Path to the SPIRE Server API socket (default "/tmp/spire-server/private/api.sock") +` +) diff --git a/cmd/spire-server/cli/logger/set_test.go b/cmd/spire-server/cli/logger/set_test.go index 36aece3f16..3b18d11c3b 100644 --- a/cmd/spire-server/cli/logger/set_test.go +++ b/cmd/spire-server/cli/logger/set_test.go @@ -9,17 +9,6 @@ import ( "github.com/spiffe/spire/cmd/spire-server/cli/logger" ) -var ( - setUsage = `Usage of logger set: - -level string - The new log level, one of (panic, fatal, error, warn, info, debug, trace, launch) - -output value - Desired output format (pretty, json); default: pretty. - -socketPath string - Path to the SPIRE Server API socket (default "/tmp/spire-server/private/api.sock") -` -) - func TestSetHelp(t *testing.T) { test := setupCliTest(t, nil, logger.NewSetCommandWithEnv) test.client.Help() diff --git a/cmd/spire-server/cli/logger/set_windows_test.go b/cmd/spire-server/cli/logger/set_windows_test.go new file mode 100644 index 0000000000..7a561d3986 --- /dev/null +++ b/cmd/spire-server/cli/logger/set_windows_test.go @@ -0,0 +1,14 @@ +//go:build windows + +package logger_test + +var ( + setUsage = `Usage of logger set: + -level string + The new log level, one of (panic, fatal, error, warn, info, debug, trace, launch) + -namedPipeName string + Pipe name of the SPIRE Server API named pipe (default "\\spire-server\\private\\api") + -output value + Desired output format (pretty, json); default: pretty. +` +)