Work to align to Agustin's design goals.

Signed-off-by: Edwin Buck <edwbuck@gmail.com>

cmd/spire-server/cli/validate/validate.go

@@ -68,20 +68,24 @@ func (c *validateCommand) Run(args []string) int {
 		ctx = context.Background()
 	}
 
-	err = s.Run(ctx)
+	result, err := s.ValidateConfig(ctx)
 	if err != nil {
-		config.Log.WithError(err).Error("Validation failed: validation server crashed")
+		config.Log.WithError(err).Errorf("Validation failed: %s", err)
 		return 1
 	}
 
+	// required to add Valid json output flag
 	err = c.printer.PrintStruct(&validateResult{
-		Valid: config.ValidationError == "",
-		Error: config.ValidationError,
-		Notes: config.ValidationNotes,
+		Valid: result.ValidationError == "",
+		Error: result.ValidationError,
+		Notes: result.ValidationNotes,
 	})
+
+
 	if err != nil {
 		return 1
 	}
+
 	return 0
 }
 
@@ -98,7 +102,7 @@ func (c *validateCommand) prettyPrintValidate(env *commoncli.Env, results ...any
 			return errors.New("unexpected type")
 		}
 		// pretty print error section
-		if !result.Valid {
+		if result.Error != "" {
 			if err := env.Printf("Validation error:\n"); err != nil {
 				return err
 			}

pkg/common/catalog/catalog.go

@@ -269,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,

pkg/common/validation/validationResult.go

@@ -0,0 +1,61 @@
+package validation
+
+import (
+	"errors"
+	"fmt"
+)
+
+/* Captures the result of a configuration validation */
+// TODO: make ValidationError validationError
+// TODO: make ValidationNotes validationNotes
+type ValidationResult struct {
+	ValidationError string    `json:"error"`
+	ValidationNotes []string  `json:"notes"`
+}
+
+func (v *ValidationResult) Error() error {
+	if v.ValidationError != "" {
+		return errors.New(v.ValidationError)
+	}
+	return nil
+}
+
+func (v *ValidationResult) HasError() bool {
+	return v.ValidationError != ""
+}
+
+func (v *ValidationResult) ReportError(message string) {
+	if v.ValidationError == "" {
+		v.ValidationError = message
+	}
+	v.ValidationNotes = append(v.ValidationNotes, message)
+}
+
+func (v *ValidationResult) ReportErrorf(format string, params ...any) {
+	v.ReportError(fmt.Sprintf(format, params...))
+}
+
+func (v *ValidationResult) ReportInfo(message string) {
+	v.ValidationNotes = append(v.ValidationNotes, message)
+}
+
+func (v *ValidationResult) ReportInfof(format string, params ...any) {
+	v.ReportInfo(fmt.Sprintf(format, params...))
+}
+
+func (v *ValidationResult) Merge(other ValidationResult) {
+	if v.ValidationError == "" {
+		v.ValidationError = other.ValidationError
+	}
+	v.ValidationNotes = append(v.ValidationNotes, other.ValidationNotes...)
+}
+
+func (v *ValidationResult) MergeWithFormat(format string, other ValidationResult) {
+	if v.ValidationError == "" && other.ValidationError != "" {
+		v.ValidationError = fmt.Sprintf(format, other.ValidationError)
+	}
+	for _, note := range other.ValidationNotes {
+		v.ValidationNotes = append(v.ValidationNotes, fmt.Sprintf(format, note))
+	}
+}
+

pkg/server/catalog/catalog.go

@@ -2,12 +2,12 @@ package catalog
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"io"
 
 	"github.com/andres-erbsen/clock"
 	"github.com/sirupsen/logrus"
+	"github.com/sirupsen/logrus/hooks/test"
 	"github.com/spiffe/go-spiffe/v2/spiffeid"
 	"github.com/spiffe/spire-plugin-sdk/pluginsdk"
 	metricsv1 "github.com/spiffe/spire-plugin-sdk/proto/spire/hostservice/common/metrics/v1"
@@ -17,6 +17,7 @@ import (
 	"github.com/spiffe/spire/pkg/common/health"
 	"github.com/spiffe/spire/pkg/common/hostservice/metricsservice"
 	"github.com/spiffe/spire/pkg/common/telemetry"
+	"github.com/spiffe/spire/pkg/common/validation"
 	ds_telemetry "github.com/spiffe/spire/pkg/common/telemetry/server/datastore"
 	km_telemetry "github.com/spiffe/spire/pkg/common/telemetry/server/keymanager"
 	"github.com/spiffe/spire/pkg/server/cache/dscache"
@@ -208,40 +209,41 @@ func Load(ctx context.Context, config *Config) (_ *Repository, err error) {
 	return repo, nil
 }
 
-func validateSQLConfig(config *Config) (catalog.PluginConfig, PluginConfigs, error) {
+func validateSQLConfig(config *Config) (catalog.PluginConfig, PluginConfigs, validation.ValidationResult) {
+	vr := validation.ValidationResult{}
 	datastoreConfigs, pluginConfigs := config.PluginConfigs.FilterByType(dataStoreType)
+
 	switch {
 	case len(datastoreConfigs) == 0:
-		config.ReportError("expecting a DataStore plugin")
-		return catalog.PluginConfig{}, PluginConfigs(nil), errors.New("expecting a DataStore plugin")
+		vr.ReportError("expecting a DataStore plugin")
 	case len(datastoreConfigs) > 1:
-		config.ReportError("only one DataStore plugin is allowed")
-		return catalog.PluginConfig{}, PluginConfigs(nil), errors.New("only one DataStore plugin is allowed")
+		vr.ReportError("only one DataStore plugin is allowed")
 	}
 
 	datastoreConfig := datastoreConfigs[0]
 
 	if datastoreConfig.Name != ds_sql.PluginName {
-		config.ReportErrorf("pluggability for the DataStore is deprecated; only the built-in %q plugin is supported", ds_sql.PluginName)
-		return catalog.PluginConfig{}, PluginConfigs(nil), fmt.Errorf("pluggability for the DataStore is deprecated; only the built-in %q plugin is supported", ds_sql.PluginName)
+		vr.ReportErrorf("pluggability for the DataStore is deprecated; only the built-in %q plugin is supported", ds_sql.PluginName)
 	}
 	if datastoreConfig.IsExternal() {
-		config.ReportErrorf("pluggability for the DataStore is deprecated; only the built-in %q plugin is supported", ds_sql.PluginName)
-		return catalog.PluginConfig{}, PluginConfigs(nil), fmt.Errorf("pluggability for the DataStore is deprecated; only the built-in %q plugin is supported", ds_sql.PluginName)
+		vr.ReportErrorf("pluggability for the DataStore is deprecated; only the built-in %q plugin is supported", ds_sql.PluginName)
 	}
 	if datastoreConfig.DataSource == nil {
-		config.ReportError("internal: DataStore is missing a configuration data source")
+		vr.ReportError("internal: DataStore is missing a configuration data source")
 	} else if datastoreConfig.DataSource.IsDynamic() {
-		config.ReportInfo("DataStore is not reconfigurable even with a dynamic data source")
+		vr.ReportInfo("DataStore is not reconfigurable even with a dynamic data source")
 	}
 
-	return datastoreConfig, pluginConfigs, nil
+	if vr.HasError() {
+		return catalog.PluginConfig{}, PluginConfigs(nil), vr
+	}
+	return datastoreConfig, pluginConfigs, vr
 }
 
 func loadSQLDataStore(ctx context.Context, config *Config, coreConfig catalog.CoreConfig) (*ds_sql.Plugin, PluginConfigs, error) {
-	dataStoreConfig, pluginConfigs, err := validateSQLConfig(config)
-	if err != nil {
-		return nil, nil, err
+	dataStoreConfig, pluginConfigs, result := validateSQLConfig(config)
+	if result.HasError() {
+		return nil, nil, result.Error()
 	}
 	if dataStoreConfig.DataSource == nil {
 		dataStoreConfig.DataSource = catalog.FixedData("")
@@ -256,3 +258,44 @@ func loadSQLDataStore(ctx context.Context, config *Config, coreConfig catalog.Co
 	config.Log.WithField(telemetry.Reconfigurable, false).Info("Configured DataStore")
 	return ds, pluginConfigs, nil
 }
+
+func ValidateConfig(ctx context.Context, config *Config) (*validation.ValidationResult, error) {
+	validationResult := &validation.ValidationResult{}
+	if c, ok := config.PluginConfigs.Find(nodeAttestorType, jointoken.PluginName); ok && c.IsEnabled() && c.IsExternal() {
+		validationResult.ReportError("server catalog: the built-in join_token node attestor cannot be overridden by an external plugin")
+		// TODO: filter out plugin and continue
+	}
+
+	nullLogger, _ := test.NewNullLogger()
+	repo := &Repository{
+		log: nullLogger,
+	}
+	defer func() {
+		repo.Close()
+	}()
+
+	coreConfig := catalog.CoreConfig{
+		TrustDomain: config.TrustDomain,
+	}
+
+	dataStoreConfig, _, result := validateSQLConfig(config)
+	validationResult.MergeWithFormat("server catalog: %s", result)
+	if result.HasError() {
+		return validationResult, nil
+	}
+
+	ds := ds_sql.New(nullLogger)
+	dsConfigString, err := dataStoreConfig.DataSource.Load()
+	if err != nil {
+		validationResult.ReportError("server catalog: error loading sql datastore plugin config")
+	}
+	resp, err := ds.Validate(ctx, coreConfig, dsConfigString)
+	fmt.Printf("edwin: %+v, config=%s\n", ds, dsConfigString)
+	fmt.Printf("edwin: %+v, error=%+v\n", resp, err)
+
+	if err != nil {
+		validationResult.ReportError("server catalog: error validating sql datastore plugin config")
+	}
+
+	return validationResult, nil
+}

pkg/server/datastore/sqlstore/sqlstore.go

@@ -859,7 +859,7 @@ func (ds *Plugin) Configure(_ context.Context, _ catalog.CoreConfig, hclConfigur
 }
 
 func (ds *Plugin) Validate(_ context.Context, _ catalog.CoreConfig, hclConfiguration string) (*catalog.ValidateResult, error) {
-	return nil, nil
+	return nil, fmt.Errorf("%s", "edwin")
 }
 
 func (ds *Plugin) openConnections(config *configuration) error {