Skip to content
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

Squashed commit of adding support for cli plugin validation reporting. #5665

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/spire-server/cli/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"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/upstreamauthority"
"github.com/spiffe/spire/cmd/spire-server/cli/validate"

Check failure on line 20 in cmd/spire-server/cli/cli.go

View workflow job for this annotation

GitHub Actions / lint (linux)

could not import github.com/spiffe/spire/cmd/spire-server/cli/validate (-: # github.com/spiffe/spire/cmd/spire-server/cli/validate
"github.com/spiffe/spire/cmd/spire-server/cli/x509"
"github.com/spiffe/spire/pkg/common/log"
"github.com/spiffe/spire/pkg/common/version"
Expand Down Expand Up @@ -125,7 +125,7 @@
return jwt.NewMintCommand(), nil
},
"validate": func() (cli.Command, error) {
return validate.NewValidateCommand(), nil
return validate.NewValidateCommand(ctx), nil
},
"localauthority x509 show": func() (cli.Command, error) {
return localauthority_x509.NewX509ShowCommand(), nil
Expand Down
15 changes: 9 additions & 6 deletions cmd/spire-server/cli/run/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,16 +224,16 @@ func (cmd *Command) Help() string {

// Help is a standalone function that prints a help message to writer.
// It is used by both the run and validate commands, so they can share flag usage messages.
func Help(name string, writer io.Writer) string {
_, err := parseFlags(name, []string{"-h"}, writer)
func Help(name string, writer io.Writer, options ...func(fs *flag.FlagSet)) string {
_, err := parseFlags(name, []string{"-h"}, writer, options...)
// Error is always present because -h is passed
return err.Error()
}

func LoadConfig(name string, args []string, logOptions []log.Option, output io.Writer, allowUnknownConfig bool) (*server.Config, error) {
func LoadConfig(name string, args []string, logOptions []log.Option, output io.Writer, allowUnknownConfig bool, options ...func(*flag.FlagSet)) (*server.Config, error) {
// First parse the CLI flags so we can get the config
// file path, if set
cliInput, err := parseFlags(name, args, output)
cliInput, err := parseFlags(name, args, output, options...)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -269,7 +269,7 @@ func (cmd *Command) Run(args []string) int {
// Set umask before starting up the server
common_cli.SetUmask(c.Log)

s := server.New(*c)
s := server.New(c)

ctx := cmd.ctx
if ctx == nil {
Expand Down Expand Up @@ -329,7 +329,7 @@ func ParseFile(path string, expandEnv bool) (*Config, error) {
return c, nil
}

func parseFlags(name string, args []string, output io.Writer) (*serverConfig, error) {
func parseFlags(name string, args []string, output io.Writer, options ...func(*flag.FlagSet)) (*serverConfig, error) {
flags := flag.NewFlagSet(name, flag.ContinueOnError)
flags.SetOutput(output)
c := &serverConfig{}
Expand All @@ -344,6 +344,9 @@ func parseFlags(name string, args []string, output io.Writer) (*serverConfig, er
flags.StringVar(&c.LogLevel, "logLevel", "", "'debug', 'info', 'warn', or 'error'")
flags.StringVar(&c.TrustDomain, "trustDomain", "", "The trust domain that this server belongs to")
flags.BoolVar(&c.ExpandEnv, "expandEnv", false, "Expand environment variables in SPIRE config file")
for _, option := range options {
option(flags)
}
c.addOSFlags(flags)

err := flags.Parse(args)
Expand Down
108 changes: 98 additions & 10 deletions cmd/spire-server/cli/validate/validate.go
Original file line number Diff line number Diff line change
@@ -1,42 +1,130 @@
package validate

import (
"context"
"errors"
"flag"
"fmt"

"github.com/mitchellh/cli"
"github.com/spiffe/spire/cmd/spire-server/cli/run"
commoncli "github.com/spiffe/spire/pkg/common/cli"
"github.com/spiffe/spire/pkg/common/cliprinter"
"github.com/spiffe/spire/pkg/common/log"
"github.com/spiffe/spire/pkg/server"
)

const commandName = "validate"

func NewValidateCommand() cli.Command {
return newValidateCommand(commoncli.DefaultEnv)
func NewValidateCommand(ctx context.Context) cli.Command {
return newValidateCommand(ctx, commoncli.DefaultEnv)
}

func newValidateCommand(env *commoncli.Env) *validateCommand {
func newValidateCommand(ctx context.Context, env *commoncli.Env) *validateCommand {
return &validateCommand{
env: env,
ctx: ctx,
env: env,
logOptions: []log.Option{
log.WithOutputFile("/dev/null"),
},
}
}

type validateCommand struct {
env *commoncli.Env
ctx context.Context
logOptions []log.Option
env *commoncli.Env
printer cliprinter.Printer
}

// Help prints the server cmd usage
func (c *validateCommand) Help() string {
return run.Help(commandName, c.env.Stderr)
return run.Help(commandName, c.env.Stderr, c.SetupPrinter)
}

func (c *validateCommand) Synopsis() string {
return "Validates a SPIRE server configuration file"
}

func (c *validateCommand) SetupPrinter(flags *flag.FlagSet) {
cliprinter.AppendFlagWithCustomPretty(&c.printer, flags, c.env, c.prettyPrintValidate)
}

func (c *validateCommand) Run(args []string) int {
if _, err := run.LoadConfig(commandName, args, nil, c.env.Stderr, false); err != nil {
// Ignore error since a failure to write to stderr cannot very well be reported
_ = c.env.ErrPrintf("SPIRE server configuration file is invalid: %v\n", err)
config, err := run.LoadConfig(commandName, args, c.logOptions, c.env.Stderr, false, c.SetupPrinter)
if err != nil {
_, _ = fmt.Fprintln(c.env.Stderr, err)
return 1
}
_ = c.env.Println("SPIRE server configuration file is valid.")
config.ValidateOnly = true
edwbuck marked this conversation as resolved.
Show resolved Hide resolved

// Set umask before starting up the server
commoncli.SetUmask(config.Log)

s := server.New(config)

ctx := c.ctx
if ctx == nil {
ctx = context.Background()
}

result, err := s.ValidateConfig(ctx)

Check failure on line 71 in cmd/spire-server/cli/validate/validate.go

View workflow job for this annotation

GitHub Actions / unit-test (macos-latest)

s.ValidateConfig undefined (type *"github.com/spiffe/spire/pkg/server".Server has no field or method ValidateConfig)

Check failure on line 71 in cmd/spire-server/cli/validate/validate.go

View workflow job for this annotation

GitHub Actions / unit-test (ubuntu-22.04)

s.ValidateConfig undefined (type *"github.com/spiffe/spire/pkg/server".Server has no field or method ValidateConfig)

Check failure on line 71 in cmd/spire-server/cli/validate/validate.go

View workflow job for this annotation

GitHub Actions / lint (linux)

s.ValidateConfig undefined (type *"github.com/spiffe/spire/pkg/server".Server has no field or method ValidateConfig)) (typecheck)

Check failure on line 71 in cmd/spire-server/cli/validate/validate.go

View workflow job for this annotation

GitHub Actions / lint (linux)

s.ValidateConfig undefined (type *"github.com/spiffe/spire/pkg/server".Server has no field or method ValidateConfig)

Check failure on line 71 in cmd/spire-server/cli/validate/validate.go

View workflow job for this annotation

GitHub Actions / artifacts (windows)

s.ValidateConfig undefined (type *"github.com/spiffe/spire/pkg/server".Server has no field or method ValidateConfig)

Check failure on line 71 in cmd/spire-server/cli/validate/validate.go

View workflow job for this annotation

GitHub Actions / unit-test (linux with race detection)

s.ValidateConfig undefined (type *"github.com/spiffe/spire/pkg/server".Server has no field or method ValidateConfig)

Check failure on line 71 in cmd/spire-server/cli/validate/validate.go

View workflow job for this annotation

GitHub Actions / unit-test (windows)

s.ValidateConfig undefined (type *"github.com/spiffe/spire/pkg/server".Server has no field or method ValidateConfig)
if err != nil {
config.Log.WithError(err).Errorf("Validation failed: %s", err)
return 1
}

// required to add Valid json output flag
err = c.printer.PrintStruct(&validateResult{
Valid: result.ValidationError == "",
Error: result.ValidationError,
Notes: result.ValidationNotes,
})


if err != nil {
return 1
}

return 0
}

type validateResult struct {
Valid bool `json:"valid"`
Error string `json:"error"`
Notes []string `json:"notes"`
}

func (c *validateCommand) prettyPrintValidate(env *commoncli.Env, results ...any) error {
if resultInterface, ok := results[0].([]any); ok {
result, ok := resultInterface[0].(*validateResult)
if !ok {
return errors.New("unexpected type")
}
// pretty print error section
if result.Error != "" {
if err := env.Printf("Validation error:\n"); err != nil {
return err
}
if err := env.Printf(" %s\n", result.Error); err != nil {
return err
}
}
// pretty print notes section
if len(result.Notes) < 1 {
if err := env.Printf("No validation notes\n"); err != nil {
return err
}
return nil
}
if err := env.Printf("Validation notes:\n"); err != nil {
return err
}
for _, note := range result.Notes {
if err := env.Printf(" %s\n", note); err != nil {
return err
}
}
}
return cliprinter.ErrInternalCustomPrettyFunc
}
15 changes: 13 additions & 2 deletions cmd/spire-server/cli/validate/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import (
"bytes"
"context"
"testing"

"github.com/mitchellh/cli"
Expand Down Expand Up @@ -31,11 +32,11 @@
s.stdout = new(bytes.Buffer)
s.stderr = new(bytes.Buffer)

s.cmd = newValidateCommand(&common_cli.Env{
s.cmd = newValidateCommand(context.Background(), &common_cli.Env{
Stdin: s.stdin,
Stdout: s.stdout,
Stderr: s.stderr,
})
}, nil)

Check failure on line 39 in cmd/spire-server/cli/validate/validate_test.go

View workflow job for this annotation

GitHub Actions / lint (linux)

too many arguments in call to newValidateCommand
}

func (s *ValidateSuite) TestSynopsis() {
Expand All @@ -53,3 +54,13 @@
s.Equal("", s.stdout.String(), "stdout")
s.Contains(s.stderr.String(), "flag provided but not defined: -badflag")
}

func (s *ValidateSuite) TestValidate() {
code := s.cmd.Run([]string{"-config", "../../../../test/fixture/config/server_good_basic.conf"})
s.Equal(0, code, "exit code")
}

func (s *ValidateSuite) TestValidateFails() {
code := s.cmd.Run([]string{"-config", "../../../../test/fixture/config/server_bad_disk_keymanager_plugin.conf"})
s.Equal(1, code, "exit code")
}
2 changes: 1 addition & 1 deletion pkg/agent/catalog/catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func Load(ctx context.Context, config Config) (_ *Repository, err error) {
repo := &Repository{
log: config.Log,
}
repo.catalog, err = catalog.Load(ctx, catalog.Config{
repo.catalog, err = catalog.Load(ctx, &catalog.Config{
Log: config.Log,
CoreConfig: catalog.CoreConfig{
TrustDomain: config.TrustDomain,
Expand Down
87 changes: 74 additions & 13 deletions pkg/common/catalog/catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,34 @@ type Config struct {

// CoreConfig is the core configuration provided to each plugin.
CoreConfig CoreConfig

// Validate plugins only
ValidateOnly bool

// Validation findings
ValidationNotes []string

// First validation error
ValidationError string
}

func (c *Config) ReportInfo(message string) {
c.ValidationNotes = append(c.ValidationNotes, message)
}

func (c *Config) ReportInfof(message string, args ...any) {
c.ReportInfo(fmt.Sprintf(message, args...))
}

func (c *Config) ReportError(message string) {
if c.ValidationError == "" {
c.ValidationError = message
}
c.ValidationNotes = append(c.ValidationNotes, message)
}

func (c *Config) ReportErrorf(message string, args ...any) {
c.ReportError(fmt.Sprintf(message, args...))
}

type Catalog struct {
Expand All @@ -129,7 +157,7 @@ func (c *Catalog) Close() error {
// given catalog are considered invalidated. If any plugin fails to load or
// configure, all plugins are unloaded, the catalog is cleared, and the
// function returns an error.
func Load(ctx context.Context, config Config, repo Repository) (_ *Catalog, err error) {
func Load(ctx context.Context, config *Config, repo Repository) (_ *Catalog, err error) {
closers := make(closerGroup, 0)
defer func() {
// If loading fails, clear out the catalog and close down all plugins
Expand All @@ -149,6 +177,7 @@ func Load(ctx context.Context, config Config, repo Repository) (_ *Catalog, err
if err != nil {
return nil, err
}

serviceRepos, err := makeBindableServiceRepos(repo.Services())
if err != nil {
return nil, err
Expand All @@ -162,8 +191,11 @@ func Load(ctx context.Context, config Config, repo Repository) (_ *Catalog, err

pluginRepo, ok := pluginRepos[pluginConfig.Type]
if !ok {
pluginLog.Error("Unsupported plugin type")
return nil, fmt.Errorf("unsupported plugin type %q", pluginConfig.Type)
config.ReportErrorf("common catalog: Unsupported plugin %q of type %q", pluginConfig.Name, pluginConfig.Type)
if !config.ValidateOnly {
return nil, fmt.Errorf("unsupported plugin type %q", pluginConfig.Type)
}
continue
}

if pluginConfig.Disabled {
Expand All @@ -173,8 +205,12 @@ func Load(ctx context.Context, config Config, repo Repository) (_ *Catalog, err

plugin, err := loadPlugin(ctx, pluginRepo.BuiltIns(), pluginConfig, pluginLog, config.HostServices)
if err != nil {
config.ReportErrorf("commmon catalog: plugin %q failed to load", pluginConfig.Name)
pluginLog.WithError(err).Error("Failed to load plugin")
return nil, fmt.Errorf("failed to load plugin %q: %w", pluginConfig.Name, err)
if !config.ValidateOnly {
return nil, fmt.Errorf("failed to load plugin %q: %w", pluginConfig.Name, err)
}
continue
}

// Add the plugin to the closers even though it has not been completely
Expand All @@ -185,17 +221,31 @@ func Load(ctx context.Context, config Config, repo Repository) (_ *Catalog, err

configurer, err := plugin.bindRepos(pluginRepo, serviceRepos)
if err != nil {
config.ReportErrorf("commmon catalog: failed to bind plugin %q", pluginConfig.Name)
pluginLog.WithError(err).Error("Failed to bind plugin")
return nil, fmt.Errorf("failed to bind plugin %q: %w", pluginConfig.Name, err)
if !config.ValidateOnly {
return nil, fmt.Errorf("failed to bind plugin %q: %w", pluginConfig.Name, err)
}
}

reconfigurer, err := configurePlugin(ctx, pluginLog, config.CoreConfig, configurer, pluginConfig.DataSource)
if err != nil {
pluginLog.WithError(err).Error("Failed to configure plugin")
return nil, fmt.Errorf("failed to configure plugin %q: %w", pluginConfig.Name, err)
}
if reconfigurer != nil {
reconfigurers = append(reconfigurers, reconfigurer)
if !config.ValidateOnly {
reconfigurer, err := configurePlugin(ctx, pluginLog, config.CoreConfig, configurer, pluginConfig.DataSource)
if err != nil {
pluginLog.WithError(err).Error("Failed to configure plugin")
return nil, fmt.Errorf("failed to configure plugin %q: %w", pluginConfig.Name, err)
}
if reconfigurer != nil {
reconfigurers = append(reconfigurers, reconfigurer)
}
} else {
result, _ := validatePlugin(ctx, pluginLog, config.CoreConfig, configurer, pluginConfig.DataSource)
for _, note := range result.Notes {
if note == result.Error {
config.ReportErrorf("plugin %s(%q): %s", pluginConfig.Type, pluginConfig.Name, note)
} else {
config.ReportInfof("plugin %s(%q): %s", pluginConfig.Type, pluginConfig.Name, note)
}
}
}

pluginLog.Info("Plugin loaded")
Expand All @@ -205,7 +255,11 @@ func Load(ctx context.Context, config Config, repo Repository) (_ *Catalog, err
// Make sure all plugin constraints are satisfied
for pluginType, pluginRepo := range pluginRepos {
if err := pluginRepo.Constraints().Check(pluginCounts[pluginType]); err != nil {
return nil, fmt.Errorf("plugin type %q constraint not satisfied: %w", pluginType, err)
config.ReportErrorf("commmon catalog: plugin type %q constraint violation: %s", pluginType, err.Error())
if !config.ValidateOnly {
return nil, fmt.Errorf("plugin type %q constraint not satisfied: %w", pluginType, err)
}
continue
}
}

Expand All @@ -215,6 +269,13 @@ func Load(ctx context.Context, config Config, repo Repository) (_ *Catalog, err
}, nil
}

func GetPluginConfigString(c PluginConfig) (string, error) {
if c.DataSource == nil {
return "", nil
}
return c.DataSource.Load()
}

func makePluginLog(log logrus.FieldLogger, pluginConfig PluginConfig) logrus.FieldLogger {
return log.WithFields(logrus.Fields{
telemetry.PluginName: pluginConfig.Name,
Expand Down
2 changes: 1 addition & 1 deletion pkg/common/catalog/catalog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ func testLoad(t *testing.T, pluginPath string, tt loadTest) {
tt.mutateServiceRepo(serviceRepo)
}

cat, err := catalog.Load(context.Background(), config, repo)
cat, err := catalog.Load(context.Background(), &config, repo)
if cat != nil {
defer func() {
cat.Close()
Expand Down
Loading
Loading