-
Notifications
You must be signed in to change notification settings - Fork 486
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Post launch Log level control for the Server #4880
Merged
Merged
Changes from all commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
b1c09c0
Initial support for dynamically adjustable log levels.
edwbuck 660ef92
Fixed logical error in comment.
edwbuck 0d9355a
Functional for get and set logger commands.
edwbuck adef016
Adjustments to improve JSON output formatting.
edwbuck 57f3da0
Realign to final spire-api-sdk api.
edwbuck e274962
Rework server commands to use new API
edwbuck 25ef540
Fix off-by one log level setting, introduced with new API change.
edwbuck 801e8af
Fix the issue of not configuring the log.Logger (from pkg) or
edwbuck 6d912a8
Make the service fields private, small renames, add a type.
edwbuck 0355b90
Add the RootLog field to the config in the endpoints_test.go.
edwbuck 5aa92ff
Minor adjustements to conform with go's linting policies.
edwbuck 10ac0f9
Fixes for small code review points. Additions for testing under
edwbuck 2c35b8b
Flipped all testing of various access methods to true until issue 4940
edwbuck daeb76d
Add Logger GRPC Service unit testing.
edwbuck b5c6058
Fix lint issues.
edwbuck 9966197
Fix unit test that was unnoticed as the test was cached.
edwbuck d07317e
Fix unit tests on windows.
edwbuck e4d675f
goimport get_posix_test.go
edwbuck d8ea93d
goimport format the windows test data in get_windows_test.go
edwbuck 1c5c144
Fix windows unit testing of logger set cli command.
edwbuck File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
package logger | ||
|
||
import ( | ||
"context" | ||
"flag" | ||
"fmt" | ||
|
||
"github.com/mitchellh/cli" | ||
api "github.com/spiffe/spire-api-sdk/proto/spire/api/server/logger/v1" | ||
"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 (c *getCommand) prettyPrintLogger(env *commoncli.Env, results ...any) error { | ||
return PrettyPrintLogger(env, results...) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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") | ||
` | ||
) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,178 @@ | ||
package logger_test | ||
|
||
import ( | ||
"errors" | ||
"testing" | ||
|
||
"github.com/stretchr/testify/require" | ||
|
||
"github.com/spiffe/spire-api-sdk/proto/spire/api/types" | ||
"github.com/spiffe/spire/cmd/spire-server/cli/logger" | ||
) | ||
|
||
func TestGetHelp(t *testing.T) { | ||
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) { | ||
cmd := logger.NewGetCommand() | ||
require.Equal(t, "Gets the logger details", cmd.Synopsis()) | ||
} | ||
|
||
func TestGet(t *testing.T) { | ||
for _, tt := range []struct { | ||
name string | ||
// server state | ||
server *mockLoggerServer | ||
// input | ||
args []string | ||
// expected items | ||
expectReturnCode int | ||
expectStdout string | ||
expectStderr string | ||
}{ | ||
{ | ||
name: "configured to info, set to info, using pretty output", | ||
args: []string{"-output", "pretty"}, | ||
server: &mockLoggerServer{ | ||
returnLogger: &types.Logger{ | ||
CurrentLevel: types.LogLevel_INFO, | ||
LaunchLevel: types.LogLevel_INFO, | ||
}, | ||
}, | ||
expectReturnCode: 0, | ||
expectStdout: `Logger Level : info | ||
Launch Level : info | ||
|
||
`, | ||
}, | ||
{ | ||
name: "configured to debug, set to warn, using pretty output", | ||
args: []string{"-output", "pretty"}, | ||
server: &mockLoggerServer{ | ||
returnLogger: &types.Logger{ | ||
CurrentLevel: types.LogLevel_WARN, | ||
LaunchLevel: types.LogLevel_DEBUG, | ||
}, | ||
}, | ||
expectReturnCode: 0, | ||
expectStdout: `Logger Level : warning | ||
Launch Level : debug | ||
|
||
`, | ||
}, | ||
{ | ||
name: "configured to error, set to trace, using pretty output", | ||
args: []string{"-output", "pretty"}, | ||
server: &mockLoggerServer{ | ||
returnLogger: &types.Logger{ | ||
CurrentLevel: types.LogLevel_TRACE, | ||
LaunchLevel: types.LogLevel_ERROR, | ||
}, | ||
}, | ||
expectReturnCode: 0, | ||
expectStdout: `Logger Level : trace | ||
Launch Level : error | ||
|
||
`, | ||
}, | ||
{ | ||
name: "configured to panic, set to fatal, using pretty output", | ||
args: []string{"-output", "pretty"}, | ||
server: &mockLoggerServer{ | ||
returnLogger: &types.Logger{ | ||
CurrentLevel: types.LogLevel_FATAL, | ||
LaunchLevel: types.LogLevel_PANIC, | ||
}, | ||
}, | ||
expectReturnCode: 0, | ||
expectStdout: `Logger Level : fatal | ||
Launch Level : panic | ||
|
||
`, | ||
}, | ||
{ | ||
name: "configured to info, set to info, using json output", | ||
args: []string{"-output", "json"}, | ||
server: &mockLoggerServer{ | ||
returnLogger: &types.Logger{ | ||
CurrentLevel: types.LogLevel_INFO, | ||
LaunchLevel: types.LogLevel_INFO, | ||
}, | ||
}, | ||
expectReturnCode: 0, | ||
expectStdout: `{"current_level":"INFO","launch_level":"INFO"} | ||
`, | ||
}, | ||
{ | ||
name: "configured to debug, set to warn, using json output", | ||
args: []string{"-output", "json"}, | ||
server: &mockLoggerServer{ | ||
returnLogger: &types.Logger{ | ||
CurrentLevel: types.LogLevel_WARN, | ||
LaunchLevel: types.LogLevel_DEBUG, | ||
}, | ||
}, | ||
expectReturnCode: 0, | ||
expectStdout: `{"current_level":"WARN","launch_level":"DEBUG"} | ||
`, | ||
}, | ||
{ | ||
name: "configured to error, set to trace, using json output", | ||
args: []string{"-output", "json"}, | ||
server: &mockLoggerServer{ | ||
returnLogger: &types.Logger{ | ||
CurrentLevel: types.LogLevel_TRACE, | ||
LaunchLevel: types.LogLevel_ERROR, | ||
}, | ||
}, | ||
expectReturnCode: 0, | ||
expectStdout: `{"current_level":"TRACE","launch_level":"ERROR"} | ||
`, | ||
}, | ||
{ | ||
name: "configured to panic, set to fatal, using json output", | ||
args: []string{"-output", "json"}, | ||
server: &mockLoggerServer{ | ||
returnLogger: &types.Logger{ | ||
CurrentLevel: types.LogLevel_FATAL, | ||
LaunchLevel: types.LogLevel_PANIC, | ||
}, | ||
}, | ||
expectReturnCode: 0, | ||
expectStdout: `{"current_level":"FATAL","launch_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: internal error: returned current log level is undefined; please report this as a bug | ||
`, | ||
}, | ||
} { | ||
t.Run(tt.name, func(t *testing.T) { | ||
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) | ||
}) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
//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. | ||
` | ||
) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,110 @@ | ||||||
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" | ||||||
) | ||||||
|
||||||
// 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 *types.LogLevel | ||||||
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.NewLevel | ||||||
return s.returnLogger, s.returnErr | ||||||
} | ||||||
|
||||||
func (s *mockLoggerServer) ResetLogLevel(_ context.Context, _ *loggerv1.ResetLogLevelRequest) (*types.Logger, error) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
s.receivedSetValue = nil | ||||||
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() | ||||||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.