diff --git a/asconf/asconf.go b/asconf/asconf.go index 632084f..c88a552 100644 --- a/asconf/asconf.go +++ b/asconf/asconf.go @@ -2,7 +2,10 @@ package asconf import ( "bytes" + "errors" "fmt" + "sort" + "strings" "github.com/aerospike/aerospike-management-lib/asconfig" "github.com/go-logr/logr" @@ -59,21 +62,85 @@ func NewAsconf(source []byte, srcFmt, outFmt Format, aerospikeVersion string, lo return ac, err } -func (ac *asconf) Validate() error { +type ValidationErr struct { + asconfig.ValidationErr +} + +type VErrSlice []ValidationErr + +func (a VErrSlice) Len() int { return len(a) } +func (a VErrSlice) Swap(i, j int) { a[i], a[j] = a[j], a[i] } +func (a VErrSlice) Less(i, j int) bool { return strings.Compare(a[i].Error(), a[j].Error()) == -1 } + +func (o ValidationErr) Error() string { + verrTemplate := "description: %s, error-type: %s" + return fmt.Sprintf(verrTemplate, o.Description, o.ErrType) +} + +type ValidationErrors struct { + Errors VErrSlice +} + +func (o ValidationErrors) Error() string { + errorsByContext := map[string]VErrSlice{} + + sort.Sort(o.Errors) + + for _, err := range o.Errors { + errorsByContext[err.Context] = append(errorsByContext[err.Context], err) + } + + contexts := []string{} + for ctx := range errorsByContext { + contexts = append(contexts, ctx) + } + + sort.Strings(contexts) + + errString := "" + + for _, ctx := range contexts { + errString += fmt.Sprintf("context: %s\n", ctx) + + errList := errorsByContext[ctx] + for _, err := range errList { + + // filter "Must validate one and only one schema " errors + // I have never seen a useful one and they seem to always be + // accompanied by another more useful error that will be displayed + if err.ErrType == "number_one_of" { + continue + } + + errString += fmt.Sprintf("\t- %s\n", err.Error()) + } + } + + return errString +} + +// Validate validates the parsed configuration in ac against +// the Aerospike schema matching ac.aerospikeVersion. +// ValidationErrors is not nil if any errors during validation occur. +// ValidationErrors Error() method outputs a human readable string of validation error details. +// error is not nil if validation, or any other type of error occurs. +func (ac *asconf) Validate() (*ValidationErrors, error) { - valid, validationErrors, err := ac.cfg.IsValid(ac.managementLibLogger, ac.aerospikeVersion) + valid, tempVerrs, err := ac.cfg.IsValid(ac.managementLibLogger, ac.aerospikeVersion) - if len(validationErrors) > 0 { - for _, e := range validationErrors { - ac.logger.Errorf("Aerospike config validation error: %+v", e) + verrs := ValidationErrors{} + for _, v := range tempVerrs { + verr := ValidationErr{ + ValidationErr: *v, } + verrs.Errors = append(verrs.Errors, verr) } - if !valid || err != nil || len(validationErrors) > 0 { - return fmt.Errorf("%w, %w", ErrConfigValidation, err) + if !valid || err != nil || len(verrs.Errors) > 0 { + return &verrs, errors.Join(ErrConfigValidation, err) } - return err + return nil, nil } func (ac *asconf) MarshalText() (text []byte, err error) { diff --git a/asconf/asconf_test.go b/asconf/asconf_test.go index 9eb7e0f..f5551df 100644 --- a/asconf/asconf_test.go +++ b/asconf/asconf_test.go @@ -155,7 +155,7 @@ func Test_asconf_Validate(t *testing.T) { src: tt.fields.src, aerospikeVersion: tt.fields.aerospikeVersion, } - if err := ac.Validate(); (err != nil) != tt.wantErr { + if _, err := ac.Validate(); (err != nil) != tt.wantErr { t.Errorf("asconf.Validate() error = %v, wantErr %v", err, tt.wantErr) } }) diff --git a/cmd/convert.go b/cmd/convert.go index cb7b6bb..7f879e8 100644 --- a/cmd/convert.go +++ b/cmd/convert.go @@ -35,7 +35,7 @@ var convertCmd = newConvertCmd() func newConvertCmd() *cobra.Command { res := &cobra.Command{ - Use: "convert [flags] ", + Use: "convert [flags] ", Short: "Convert between yaml and Aerospike config format.", Long: `Convert is used to convert between yaml and aerospike configuration files. Input files are converted to their opposite format, yaml -> conf, conf -> yaml. @@ -115,9 +115,9 @@ func newConvertCmd() *cobra.Command { } if !force { - err = conf.Validate() - if err != nil { - return err + verrs, err := conf.Validate() + if err != nil || verrs != nil { + return errors.Join(err, verrs) } } @@ -222,7 +222,8 @@ func newConvertCmd() *cobra.Command { // flags and configuration settings // aerospike-version is marked required in this cmd's PreRun if the --force flag is not provided - res.Flags().StringP("aerospike-version", "a", "", "Aerospike server version to validate the configuration file for. Ex: 6.2.0.\nThe first 3 digits of the Aerospike version number are required.\nThis option is required unless --force is used") + commonFlags := getCommonFlags() + res.Flags().AddFlagSet(commonFlags) res.Flags().BoolP("force", "f", false, "Override checks for supported server version and config validation") res.Flags().StringP("output", "o", os.Stdout.Name(), "File path to write output to") diff --git a/cmd/diff_test.go b/cmd/diff_test.go index bb1da13..596c516 100644 --- a/cmd/diff_test.go +++ b/cmd/diff_test.go @@ -13,7 +13,7 @@ type runTestDiff struct { expectError bool } -var testArgs = []runTestDiff{ +var testDiffArgs = []runTestDiff{ { flags: []string{}, arguments: []string{"not_enough_args"}, @@ -64,7 +64,7 @@ var testArgs = []runTestDiff{ func TestRunEDiff(t *testing.T) { cmd := diffCmd - for i, test := range testArgs { + for i, test := range testDiffArgs { cmd.ParseFlags(test.flags) err := cmd.RunE(cmd, test.arguments) if test.expectError == (err == nil) { diff --git a/cmd/utils.go b/cmd/utils.go index 3c67cd1..e65639f 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -8,8 +8,17 @@ import ( "github.com/aerospike/asconfig/asconf" "github.com/spf13/cobra" + "github.com/spf13/pflag" ) +// common flags +func getCommonFlags() *pflag.FlagSet { + res := &pflag.FlagSet{} + res.StringP("aerospike-version", "a", "", "Aerospike server version to validate the configuration file for. Ex: 6.2.0.\nThe first 3 digits of the Aerospike version number are required.\nThis option is required unless --force is used.") + + return res +} + // getConfFileFormat guesses the format of an input config file // based on file extension and the --format flag of the cobra command // this function implements the defaults scheme for file formats in asconfig diff --git a/cmd/validate.go b/cmd/validate.go new file mode 100644 index 0000000..b193d47 --- /dev/null +++ b/cmd/validate.go @@ -0,0 +1,108 @@ +package cmd + +import ( + "errors" + "fmt" + "os" + + "github.com/aerospike/asconfig/asconf" + "github.com/spf13/cobra" +) + +const ( + validateArgMax = 1 +) + +var ( + errValidateTooManyArguments = fmt.Errorf("expected a maximum of %d arguments", convertArgMax) +) + +func init() { + rootCmd.AddCommand(validateCmd) +} + +var validateCmd = newValidateCmd() + +func newValidateCmd() *cobra.Command { + res := &cobra.Command{ + Use: "validate [flags] ", + Short: "Validate an Aerospike configuration file.", + Long: `Validate an Aerospike configuration file in any supported format + against a versioned Aerospike configuration schema. + If a file passes validation nothing is output, otherwise errors + indicating problems with the configuration file are shown. + If a file path is not provided, validate reads from stdin. + Ex: asconfig validate --aerospike-version 7.0.0 aerospike.conf`, + RunE: func(cmd *cobra.Command, args []string) error { + logger.Debug("Running validate command") + + if len(args) > validateArgMax { + return errValidateTooManyArguments + } + + // read stdin by default + var srcPath string + if len(args) == 0 { + srcPath = os.Stdin.Name() + } else { + srcPath = args[0] + } + + version, err := cmd.Flags().GetString("aerospike-version") + if err != nil { + return err + } + + logger.Debugf("Processing flag aerospike-version value=%s", version) + + srcFormat, err := getConfFileFormat(srcPath, cmd) + if err != nil { + return err + } + + logger.Debugf("Processing flag format value=%v", srcFormat) + + fdata, err := os.ReadFile(srcPath) + if err != nil { + return err + } + + conf, err := asconf.NewAsconf( + fdata, + srcFormat, + // we aren't converting to anything so set + // output format to Invalid as a place holder + asconf.Invalid, + version, + logger, + managementLibLogger, + ) + + if err != nil { + return err + } + + verrs, err := conf.Validate() + if verrs != nil { + // force validation errors to be written to stdout + // so they can more easily be grepd etc. + cmd.Print(verrs.Error()) + return errors.Join(asconf.ErrConfigValidation, SilentError) + } + if err != nil { + return err + } + + return err + }, + } + + // flags and configuration settings + commonFlags := getCommonFlags() + res.Flags().AddFlagSet(commonFlags) + res.MarkFlagRequired("aerospike-version") + + res.Version = VERSION + + return res +} diff --git a/cmd/validate_test.go b/cmd/validate_test.go new file mode 100644 index 0000000..6964af6 --- /dev/null +++ b/cmd/validate_test.go @@ -0,0 +1,66 @@ +//go:build unit +// +build unit + +package cmd + +import ( + "testing" +) + +type runTestValidate struct { + flags []string + arguments []string + expectError bool +} + +var testValidateArgs = []runTestValidate{ + { + flags: []string{}, + arguments: []string{"too", "many", "args"}, + expectError: true, + }, + { + // missing arg to -a-aerospike-version + flags: []string{"--aerospike-version"}, + arguments: []string{"../testdata/sources/all_flash_cluster_cr.yaml"}, + expectError: true, + }, + { + flags: []string{"--aerospike-version"}, + arguments: []string{"./bad_extension.ymml"}, + expectError: true, + }, + { + flags: []string{"--aerospike-version", "bad_version"}, + arguments: []string{"../testdata/sources/all_flash_cluster_cr.yaml"}, + expectError: true, + }, + { + flags: []string{"--aerospike-version", "6.4.0"}, + arguments: []string{"./fake_file.yaml"}, + expectError: true, + }, + { + flags: []string{"--aerospike-version", "6.4.0"}, + arguments: []string{"../testdata/cases/server64/server64.yaml"}, + expectError: false, + }, + { + flags: []string{"--log-level", "debug", "--aerospike-version", "7.0.0"}, + arguments: []string{"../testdata/cases/server70/server70.conf"}, + expectError: false, + }, +} + +func TestRunEValidate(t *testing.T) { + cmd := validateCmd + + for i, test := range testValidateArgs { + cmd.Parent().ParseFlags(test.flags) + cmd.ParseFlags(test.flags) + err := cmd.RunE(cmd, test.arguments) + if test.expectError == (err == nil) { + t.Fatalf("case: %d, expectError: %v does not match err: %v", i, test.expectError, err) + } + } +} diff --git a/go.mod b/go.mod index fdc6c85..8f16b0c 100644 --- a/go.mod +++ b/go.mod @@ -10,6 +10,7 @@ require ( github.com/opencontainers/image-spec v1.0.2 github.com/sirupsen/logrus v1.9.3 github.com/spf13/cobra v1.7.0 + github.com/spf13/pflag v1.0.5 gopkg.in/yaml.v3 v3.0.1 ) @@ -25,7 +26,6 @@ require ( github.com/opencontainers/go-digest v1.0.0 // indirect github.com/pkg/errors v0.9.1 // indirect github.com/qdm12/reprint v0.0.0-20200326205758-722754a53494 // indirect - github.com/spf13/pflag v1.0.5 // indirect github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb // indirect github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect github.com/xeipuuv/gojsonschema v1.2.0 // indirect diff --git a/go.sum b/go.sum index 11aeebe..46549fa 100644 --- a/go.sum +++ b/go.sum @@ -1,10 +1,6 @@ github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 h1:UQHMgLO+TxOElx5B5HZ4hJQsoJ/PvUvKRhJHDQXO8P8= github.com/Microsoft/go-winio v0.6.1 h1:9/kr64B9VUZrLm5YYwbGtUJnMgqWVOdUAXu6Migciow= github.com/Microsoft/go-winio v0.6.1/go.mod h1:LRdKpFKfdobln8UmuiYcKPot9D2v6svN5+sAH+4kjUM= -github.com/aerospike/aerospike-management-lib v0.0.0-20231025222637-439d643badb8 h1:DhmnlRVu4CURbZhA6oTSyTUSKaG3XWYOviEnyxOzy0g= -github.com/aerospike/aerospike-management-lib v0.0.0-20231025222637-439d643badb8/go.mod h1:O4v2oGl4VjG9KwYJoSVEwZXv1PUB4ioKAsrm2tczJPQ= -github.com/aerospike/aerospike-management-lib v0.0.0-20231025224657-765f71b4994d h1:WqKtqOqdZ41/WvbPV/3tGa4KNXZOOTdhwz/K1+P2CuI= -github.com/aerospike/aerospike-management-lib v0.0.0-20231025224657-765f71b4994d/go.mod h1:O4v2oGl4VjG9KwYJoSVEwZXv1PUB4ioKAsrm2tczJPQ= github.com/aerospike/aerospike-management-lib v0.0.0-20231106202816-b2438dbb7e03 h1:Od7oCSBCTfpRmk73fbNGXA53U1in38iNSklCEwMMZFc= github.com/aerospike/aerospike-management-lib v0.0.0-20231106202816-b2438dbb7e03/go.mod h1:O4v2oGl4VjG9KwYJoSVEwZXv1PUB4ioKAsrm2tczJPQ= github.com/bombsimon/logrusr/v4 v4.0.0 h1:Pm0InGphX0wMhPqC02t31onlq9OVyJ98eP/Vh63t1Oo= diff --git a/integration_test.go b/integration_test.go index abaca4a..1c6108a 100644 --- a/integration_test.go +++ b/integration_test.go @@ -916,3 +916,81 @@ func TestConvertStdin(t *testing.T) { } } + +type validateTest struct { + arguments []string + source string + expectError bool + expectedResult string +} + +var validateTests = []validateTest{ + { + arguments: []string{"validate", "-a", "6.2.0", filepath.Join(sourcePath, "pmem_cluster_cr.yaml")}, + expectError: false, + expectedResult: "", + source: filepath.Join(sourcePath, "pmem_cluster_cr.yaml"), + }, + { + arguments: []string{"validate", "-a", "7.0.0", filepath.Join(extraTestPath, "server64/server64.yaml")}, + expectError: true, + source: filepath.Join(extraTestPath, "server64/server64.yaml"), + expectedResult: `context: (root).namespaces.0 + - description: Additional property memory-size is not allowed, error-type: additional_property_not_allowed +context: (root).namespaces.0.storage-engine + - description: data-size is required, error-type: required +context: (root).namespaces.1 + - description: Additional property memory-size is not allowed, error-type: additional_property_not_allowed +context: (root).namespaces.1.index-type + - description: Additional property mounts-high-water-pct is not allowed, error-type: additional_property_not_allowed + - description: Additional property mounts-size-limit is not allowed, error-type: additional_property_not_allowed + - description: mounts-budget is required, error-type: required +context: (root).namespaces.1.sindex-type + - description: Additional property mounts-high-water-pct is not allowed, error-type: additional_property_not_allowed + - description: Additional property mounts-size-limit is not allowed, error-type: additional_property_not_allowed + - description: mounts-budget is required, error-type: required +context: (root).namespaces.1.storage-engine + - description: data-size is required, error-type: required +context: (root).service + - description: cluster-name is required, error-type: required +`, + }, +} + +func TestValidate(t *testing.T) { + for _, tf := range validateTests { + com := exec.Command(binPath+"/asconfig.test", tf.arguments...) + com.Env = []string{"GOCOVERDIR=" + coveragePath} + out, err := com.CombinedOutput() + if tf.expectError == (err == nil) { + t.Errorf("\nTESTCASE: %+v\nERR: %+v\n", tf.arguments, err) + } + + if string(out) != tf.expectedResult { + t.Errorf("\nTESTCASE: %+v\nACTUAL: %s\nEXPECTED: %s", tf.arguments, string(out), tf.expectedResult) + } + + } +} + +func TestStdinValidate(t *testing.T) { + for _, tf := range validateTests { + in, err := os.Open(tf.source) + if err != nil { + t.Error(err) + } + defer in.Close() + + com := exec.Command(binPath+"/asconfig.test", tf.arguments...) + com.Env = []string{"GOCOVERDIR=" + coveragePath} + com.Stdin = in + out, err := com.CombinedOutput() + if tf.expectError == (err == nil) { + t.Errorf("\nTESTCASE: %+v\nERR: %+v\n", tf.arguments, err) + } + + if string(out) != tf.expectedResult { + t.Errorf("\nTESTCASE: %+v\nACTUAL: %s\nEXPECTED: %s", tf.arguments, string(out), tf.expectedResult) + } + } +}