From f4888e8c38fca0bd0ae8f106755757f7d069cfc8 Mon Sep 17 00:00:00 2001 From: dylan Date: Thu, 2 Nov 2023 15:51:32 -0700 Subject: [PATCH 1/9] feat: tools2715 add the validate command --- cmd/convert.go | 2 +- cmd/validate.go | 103 +++++++++++++++++++++++++++++++++++++++++++ cmd/validate_test.go | 63 ++++++++++++++++++++++++++ integration_test.go | 67 ++++++++++++++++++++++++---- 4 files changed, 226 insertions(+), 9 deletions(-) create mode 100644 cmd/validate.go create mode 100644 cmd/validate_test.go diff --git a/cmd/convert.go b/cmd/convert.go index df868bb..12bddd1 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. diff --git a/cmd/validate.go b/cmd/validate.go new file mode 100644 index 0000000..1e40a19 --- /dev/null +++ b/cmd/validate.go @@ -0,0 +1,103 @@ +package cmd + +import ( + "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 JSON 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) > convertArgMax { + 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 + } + + // TODO should the validation errors be printed to stdout or stderr? + // should they come with the standard error log line? + err = conf.Validate() + if err != nil { + return err + } + + return err + }, + } + + // 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 for the configuration file. Ex: 6.2.0.\nThe first 3 digits of the Aerospike version number are required.\nThis option is required unless --force is used") + 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..b4760b3 --- /dev/null +++ b/cmd/validate_test.go @@ -0,0 +1,63 @@ +package cmd + +import ( + "testing" +) + +type runTestDiff struct { + flags []string + arguments []string + expectError bool +} + +var testArgs = []runTestDiff{ + { + 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 testArgs { + 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/integration_test.go b/integration_test.go index abaca4a..1a7d7e2 100644 --- a/integration_test.go +++ b/integration_test.go @@ -1,6 +1,3 @@ -//go:build integration -// +build integration - package main import ( @@ -67,11 +64,11 @@ func TestMain(m *testing.M) { panic(err) } - featKeyDir = os.Getenv("FEATKEY") - fmt.Println(featKeyDir) - if featKeyDir == "" { - panic("FEATKEY environement variable must be full path to a directory containing valid aerospike feature key files featuresv1.conf and featuresv2.conf of feature key format 1 and 2 respectively.") - } + // featKeyDir = os.Getenv("FEATKEY") + // fmt.Println(featKeyDir) + // if featKeyDir == "" { + // panic("FEATKEY environement variable must be full path to a directory containing valid aerospike feature key files featuresv1.conf and featuresv2.conf of feature key format 1 and 2 respectively.") + // } code := m.Run() @@ -916,3 +913,57 @@ func TestConvertStdin(t *testing.T) { } } + +type validateTest struct { + arguments []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: "", + }, + { + arguments: []string{"validate", "-a", "7.0.0", filepath.Join(extraTestPath, "server64/server64.yaml")}, + expectError: true, + expectedResult: `ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:map[group:root pidfile:/dummy/file/path1 proto-fd-max:15000 secrets-address-port:test_dns_name:4000:127.0.0.1 secrets-tls-context:tlscontext secrets-uds-path:/test/path/socket user:root] ErrType:required Context:(root).service Description:cluster-name is required Field:service} +ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:8589934592 ErrType:additional_property_not_allowed Context:(root).namespaces.0 Description:Additional property memory-size is not allowed Field:namespaces.0} +ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:map[type:memory] ErrType:number_one_of Context:(root).namespaces.0.storage-engine Description:Must validate one and only one schema (oneOf) Field:namespaces.0.storage-engine} +ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:map[type:memory] ErrType:number_one_of Context:(root).namespaces.0.storage-engine Description:Must validate one and only one schema (oneOf) Field:namespaces.0.storage-engine} +ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:map[type:memory] ErrType:required Context:(root).namespaces.0.storage-engine Description:data-size is required Field:namespaces.0.storage-engine} +ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:4294967296 ErrType:additional_property_not_allowed Context:(root).namespaces.1 Description:Additional property memory-size is not allowed Field:namespaces.1} +ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:map[mounts:[/dummy/mount/point3] mounts-high-water-pct:60 mounts-size-limit:20971520000 type:flash] ErrType:number_one_of Context:(root).namespaces.1.sindex-type Description:Must validate one and only one schema (oneOf) Field:namespaces.1.sindex-type} +ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:map[mounts:[/dummy/mount/point3] mounts-high-water-pct:60 mounts-size-limit:20971520000 type:flash] ErrType:required Context:(root).namespaces.1.sindex-type Description:mounts-budget is required Field:namespaces.1.sindex-type} +ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:60 ErrType:additional_property_not_allowed Context:(root).namespaces.1.sindex-type Description:Additional property mounts-high-water-pct is not allowed Field:namespaces.1.sindex-type} +ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:20971520000 ErrType:additional_property_not_allowed Context:(root).namespaces.1.sindex-type Description:Additional property mounts-size-limit is not allowed Field:namespaces.1.sindex-type} +ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:map[type:memory] ErrType:number_one_of Context:(root).namespaces.1.storage-engine Description:Must validate one and only one schema (oneOf) Field:namespaces.1.storage-engine} +ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:map[type:memory] ErrType:number_one_of Context:(root).namespaces.1.storage-engine Description:Must validate one and only one schema (oneOf) Field:namespaces.1.storage-engine} +ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:map[type:memory] ErrType:required Context:(root).namespaces.1.storage-engine Description:data-size is required Field:namespaces.1.storage-engine} +ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:map[mounts:[/dummy/mount/point1 /test/mount2] mounts-high-water-pct:30 mounts-size-limit:10737418240 type:flash] ErrType:number_one_of Context:(root).namespaces.1.index-type Description:Must validate one and only one schema (oneOf) Field:namespaces.1.index-type} +ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:map[mounts:[/dummy/mount/point1 /test/mount2] mounts-high-water-pct:30 mounts-size-limit:10737418240 type:flash] ErrType:required Context:(root).namespaces.1.index-type Description:mounts-budget is required Field:namespaces.1.index-type} +ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:10737418240 ErrType:additional_property_not_allowed Context:(root).namespaces.1.index-type Description:Additional property mounts-size-limit is not allowed Field:namespaces.1.index-type} +ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:30 ErrType:additional_property_not_allowed Context:(root).namespaces.1.index-type Description:Additional property mounts-high-water-pct is not allowed Field:namespaces.1.index-type} +ERRO[2023-11-02T15:33:48-07:00] error while validating config, config schema error + +`, + }, +} + +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) + } + + } +} From c8e26d97abea93c88fc185011f0094bcb385e31f Mon Sep 17 00:00:00 2001 From: dylan Date: Fri, 3 Nov 2023 15:38:32 -0700 Subject: [PATCH 2/9] reformat validation errors --- asconf/asconf.go | 69 +++++++++++++++++++++++++++++++++++++++----- cmd/convert.go | 6 ++-- cmd/diff_test.go | 4 +-- cmd/validate.go | 11 +++++-- cmd/validate_test.go | 9 ++++-- integration_test.go | 61 +++++++++++++++++++++++++++------------ 6 files changed, 123 insertions(+), 37 deletions(-) diff --git a/asconf/asconf.go b/asconf/asconf.go index 632084f..e8d8e3e 100644 --- a/asconf/asconf.go +++ b/asconf/asconf.go @@ -2,7 +2,9 @@ package asconf import ( "bytes" + "errors" "fmt" + "sort" "github.com/aerospike/aerospike-management-lib/asconfig" "github.com/go-logr/logr" @@ -59,21 +61,72 @@ func NewAsconf(source []byte, srcFmt, outFmt Format, aerospikeVersion string, lo return ac, err } -func (ac *asconf) Validate() error { +type ValidationErr struct { + asconfig.ValidationErr +} + +func (o ValidationErr) Error() string { + verrTemplate := "\x1B[1mdescription:\x1B[22m %s, \x1B[1merror-type:\x1B[22m %s" + return fmt.Sprintf(verrTemplate, o.Description, o.ErrType) +} + +type ValidationErrors struct { + Errors []ValidationErr +} + +func (o ValidationErrors) Error() string { + errorsByContext := map[string][]ValidationErr{} + + 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("\x1B[4mcontext: %s\x1B[24m\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 +} + +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/cmd/convert.go b/cmd/convert.go index 12bddd1..e1e05ee 100644 --- a/cmd/convert.go +++ b/cmd/convert.go @@ -113,9 +113,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) } } 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/validate.go b/cmd/validate.go index 1e40a19..7bec445 100644 --- a/cmd/validate.go +++ b/cmd/validate.go @@ -1,6 +1,7 @@ package cmd import ( + "errors" "fmt" "os" @@ -81,9 +82,13 @@ func newValidateCmd() *cobra.Command { return err } - // TODO should the validation errors be printed to stdout or stderr? - // should they come with the standard error log line? - err = conf.Validate() + 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 } diff --git a/cmd/validate_test.go b/cmd/validate_test.go index b4760b3..6964af6 100644 --- a/cmd/validate_test.go +++ b/cmd/validate_test.go @@ -1,16 +1,19 @@ +//go:build unit +// +build unit + package cmd import ( "testing" ) -type runTestDiff struct { +type runTestValidate struct { flags []string arguments []string expectError bool } -var testArgs = []runTestDiff{ +var testValidateArgs = []runTestValidate{ { flags: []string{}, arguments: []string{"too", "many", "args"}, @@ -52,7 +55,7 @@ var testArgs = []runTestDiff{ func TestRunEValidate(t *testing.T) { cmd := validateCmd - for i, test := range testArgs { + for i, test := range testValidateArgs { cmd.Parent().ParseFlags(test.flags) cmd.ParseFlags(test.flags) err := cmd.RunE(cmd, test.arguments) diff --git a/integration_test.go b/integration_test.go index 1a7d7e2..4d0b7bd 100644 --- a/integration_test.go +++ b/integration_test.go @@ -916,6 +916,7 @@ func TestConvertStdin(t *testing.T) { type validateTest struct { arguments []string + source string expectError bool expectedResult string } @@ -925,28 +926,30 @@ 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, - expectedResult: `ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:map[group:root pidfile:/dummy/file/path1 proto-fd-max:15000 secrets-address-port:test_dns_name:4000:127.0.0.1 secrets-tls-context:tlscontext secrets-uds-path:/test/path/socket user:root] ErrType:required Context:(root).service Description:cluster-name is required Field:service} -ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:8589934592 ErrType:additional_property_not_allowed Context:(root).namespaces.0 Description:Additional property memory-size is not allowed Field:namespaces.0} -ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:map[type:memory] ErrType:number_one_of Context:(root).namespaces.0.storage-engine Description:Must validate one and only one schema (oneOf) Field:namespaces.0.storage-engine} -ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:map[type:memory] ErrType:number_one_of Context:(root).namespaces.0.storage-engine Description:Must validate one and only one schema (oneOf) Field:namespaces.0.storage-engine} -ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:map[type:memory] ErrType:required Context:(root).namespaces.0.storage-engine Description:data-size is required Field:namespaces.0.storage-engine} -ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:4294967296 ErrType:additional_property_not_allowed Context:(root).namespaces.1 Description:Additional property memory-size is not allowed Field:namespaces.1} -ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:map[mounts:[/dummy/mount/point3] mounts-high-water-pct:60 mounts-size-limit:20971520000 type:flash] ErrType:number_one_of Context:(root).namespaces.1.sindex-type Description:Must validate one and only one schema (oneOf) Field:namespaces.1.sindex-type} -ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:map[mounts:[/dummy/mount/point3] mounts-high-water-pct:60 mounts-size-limit:20971520000 type:flash] ErrType:required Context:(root).namespaces.1.sindex-type Description:mounts-budget is required Field:namespaces.1.sindex-type} -ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:60 ErrType:additional_property_not_allowed Context:(root).namespaces.1.sindex-type Description:Additional property mounts-high-water-pct is not allowed Field:namespaces.1.sindex-type} -ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:20971520000 ErrType:additional_property_not_allowed Context:(root).namespaces.1.sindex-type Description:Additional property mounts-size-limit is not allowed Field:namespaces.1.sindex-type} -ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:map[type:memory] ErrType:number_one_of Context:(root).namespaces.1.storage-engine Description:Must validate one and only one schema (oneOf) Field:namespaces.1.storage-engine} -ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:map[type:memory] ErrType:number_one_of Context:(root).namespaces.1.storage-engine Description:Must validate one and only one schema (oneOf) Field:namespaces.1.storage-engine} -ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:map[type:memory] ErrType:required Context:(root).namespaces.1.storage-engine Description:data-size is required Field:namespaces.1.storage-engine} -ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:map[mounts:[/dummy/mount/point1 /test/mount2] mounts-high-water-pct:30 mounts-size-limit:10737418240 type:flash] ErrType:number_one_of Context:(root).namespaces.1.index-type Description:Must validate one and only one schema (oneOf) Field:namespaces.1.index-type} -ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:map[mounts:[/dummy/mount/point1 /test/mount2] mounts-high-water-pct:30 mounts-size-limit:10737418240 type:flash] ErrType:required Context:(root).namespaces.1.index-type Description:mounts-budget is required Field:namespaces.1.index-type} -ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:10737418240 ErrType:additional_property_not_allowed Context:(root).namespaces.1.index-type Description:Additional property mounts-size-limit is not allowed Field:namespaces.1.index-type} -ERRO[2023-11-02T15:33:48-07:00] Aerospike config validation error: &{Value:30 ErrType:additional_property_not_allowed Context:(root).namespaces.1.index-type Description:Additional property mounts-high-water-pct is not allowed Field:namespaces.1.index-type} -ERRO[2023-11-02T15:33:48-07:00] error while validating config, config schema error + 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: mounts-budget is required, error-type: required + 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 +context: (root).namespaces.1.sindex-type + description: mounts-budget is required, error-type: required + 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 +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 `, }, @@ -967,3 +970,25 @@ func TestValidate(t *testing.T) { } } + +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) + } + } +} From 733814e27b21a1be21c4a1d1894d50b293a5647f Mon Sep 17 00:00:00 2001 From: dylan Date: Tue, 7 Nov 2023 11:32:42 -0800 Subject: [PATCH 3/9] ci: validation integration tests --- asconf/asconf.go | 19 ++++++++++++++----- go.sum | 2 -- integration_test.go | 23 +++++++++++------------ 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/asconf/asconf.go b/asconf/asconf.go index e8d8e3e..a05a387 100644 --- a/asconf/asconf.go +++ b/asconf/asconf.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "sort" + "strings" "github.com/aerospike/aerospike-management-lib/asconfig" "github.com/go-logr/logr" @@ -65,17 +66,25 @@ 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 := "\x1B[1mdescription:\x1B[22m %s, \x1B[1merror-type:\x1B[22m %s" + verrTemplate := "description: %s, error-type: %s" return fmt.Sprintf(verrTemplate, o.Description, o.ErrType) } type ValidationErrors struct { - Errors []ValidationErr + Errors VErrSlice } func (o ValidationErrors) Error() string { - errorsByContext := map[string][]ValidationErr{} + errorsByContext := map[string]VErrSlice{} + + sort.Sort(o.Errors) for _, err := range o.Errors { errorsByContext[err.Context] = append(errorsByContext[err.Context], err) @@ -91,7 +100,7 @@ func (o ValidationErrors) Error() string { errString := "" for _, ctx := range contexts { - errString += fmt.Sprintf("\x1B[4mcontext: %s\x1B[24m\n", ctx) + errString += fmt.Sprintf("context: %s\n", ctx) errList := errorsByContext[ctx] for _, err := range errList { @@ -103,7 +112,7 @@ func (o ValidationErrors) Error() string { continue } - errString += fmt.Sprintf("\t%s\n", err.Error()) + errString += fmt.Sprintf("\t- %s\n", err.Error()) } } diff --git a/go.sum b/go.sum index fea4657..b14a322 100644 --- a/go.sum +++ b/go.sum @@ -1,8 +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/bombsimon/logrusr/v4 v4.0.0 h1:Pm0InGphX0wMhPqC02t31onlq9OVyJ98eP/Vh63t1Oo= diff --git a/integration_test.go b/integration_test.go index 4d0b7bd..fc25468 100644 --- a/integration_test.go +++ b/integration_test.go @@ -933,24 +933,23 @@ var validateTests = []validateTest{ 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 + - 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 + - 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 + - description: Additional property memory-size is not allowed, error-type: additional_property_not_allowed context: (root).namespaces.1.index-type - description: mounts-budget is required, error-type: required - 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: 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: mounts-budget is required, error-type: required - 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: 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 + - description: data-size is required, error-type: required context: (root).service - description: cluster-name is required, error-type: required - + - description: cluster-name is required, error-type: required `, }, } From 2aa3dcd6db211a7a825a48ada8dd999b1bdd3194 Mon Sep 17 00:00:00 2001 From: dylan Date: Tue, 7 Nov 2023 14:22:33 -0800 Subject: [PATCH 4/9] add back integration build tags --- integration_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/integration_test.go b/integration_test.go index fc25468..1909e57 100644 --- a/integration_test.go +++ b/integration_test.go @@ -1,3 +1,6 @@ +//go:build integration +// +build integration + package main import ( From ca654be0d96a5ad7e8d42fc1309bef62f5afa699 Mon Sep 17 00:00:00 2001 From: dylan Date: Tue, 7 Nov 2023 14:34:31 -0800 Subject: [PATCH 5/9] check validateArgMax in validate command, not convertArgMax --- cmd/validate.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cmd/validate.go b/cmd/validate.go index 7bec445..8086074 100644 --- a/cmd/validate.go +++ b/cmd/validate.go @@ -36,7 +36,7 @@ func newValidateCmd() *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { logger.Debug("Running validate command") - if len(args) > convertArgMax { + if len(args) > validateArgMax { return errValidateTooManyArguments } @@ -98,8 +98,7 @@ func newValidateCmd() *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 for the configuration file. Ex: 6.2.0.\nThe first 3 digits of the Aerospike version number are required.\nThis option is required unless --force is used") + res.Flags().StringP("aerospike-version", "a", "", "Aerospike server version for the configuration file. Ex: 6.2.0.\nThe first 3 digits of the Aerospike version number are required.") res.MarkFlagRequired("aerospike-version") res.Version = VERSION From d84dfbef6875f7065ac0a3ae5473f62f06b384ae Mon Sep 17 00:00:00 2001 From: dylan Date: Tue, 7 Nov 2023 14:44:19 -0800 Subject: [PATCH 6/9] add back featkey file integration test environment variable --- integration_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/integration_test.go b/integration_test.go index 1909e57..1c6108a 100644 --- a/integration_test.go +++ b/integration_test.go @@ -67,11 +67,11 @@ func TestMain(m *testing.M) { panic(err) } - // featKeyDir = os.Getenv("FEATKEY") - // fmt.Println(featKeyDir) - // if featKeyDir == "" { - // panic("FEATKEY environement variable must be full path to a directory containing valid aerospike feature key files featuresv1.conf and featuresv2.conf of feature key format 1 and 2 respectively.") - // } + featKeyDir = os.Getenv("FEATKEY") + fmt.Println(featKeyDir) + if featKeyDir == "" { + panic("FEATKEY environement variable must be full path to a directory containing valid aerospike feature key files featuresv1.conf and featuresv2.conf of feature key format 1 and 2 respectively.") + } code := m.Run() From be01392ee140dfb76822f22e097ff8e4e2223a53 Mon Sep 17 00:00:00 2001 From: dylan Date: Tue, 7 Nov 2023 15:17:05 -0800 Subject: [PATCH 7/9] adjust validate test for 2 return params --- asconf/asconf_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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) } }) From 6443ed26feb83ec3eadb0cb00d8fd1d1e0eabfb6 Mon Sep 17 00:00:00 2001 From: dylan Date: Tue, 14 Nov 2023 09:51:26 -0800 Subject: [PATCH 8/9] review changes --- asconf/asconf.go | 5 +++++ cmd/convert.go | 3 ++- cmd/utils.go | 9 +++++++++ cmd/validate.go | 5 +++-- 4 files changed, 19 insertions(+), 3 deletions(-) diff --git a/asconf/asconf.go b/asconf/asconf.go index a05a387..c88a552 100644 --- a/asconf/asconf.go +++ b/asconf/asconf.go @@ -119,6 +119,11 @@ func (o ValidationErrors) Error() string { 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, tempVerrs, err := ac.cfg.IsValid(ac.managementLibLogger, ac.aerospikeVersion) diff --git a/cmd/convert.go b/cmd/convert.go index e1e05ee..840a6f2 100644 --- a/cmd/convert.go +++ b/cmd/convert.go @@ -220,7 +220,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 for the configuration file. 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/utils.go b/cmd/utils.go index 3c67cd1..ab8c23e 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 for the configuration file. Ex: 6.2.0.\nThe first 3 digits of the Aerospike version number are required.") + + 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 index 8086074..b193d47 100644 --- a/cmd/validate.go +++ b/cmd/validate.go @@ -28,7 +28,7 @@ func newValidateCmd() *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 JSON schema. + 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. @@ -98,7 +98,8 @@ func newValidateCmd() *cobra.Command { } // flags and configuration settings - res.Flags().StringP("aerospike-version", "a", "", "Aerospike server version for the configuration file. Ex: 6.2.0.\nThe first 3 digits of the Aerospike version number are required.") + commonFlags := getCommonFlags() + res.Flags().AddFlagSet(commonFlags) res.MarkFlagRequired("aerospike-version") res.Version = VERSION From dfaab4ac54a93c163149ed6ca3a956e4a8ebee11 Mon Sep 17 00:00:00 2001 From: dylan Date: Tue, 14 Nov 2023 10:22:20 -0800 Subject: [PATCH 9/9] update schemas --- go.mod | 2 +- go.sum | 2 -- schema/schemas | 2 +- 3 files changed, 2 insertions(+), 4 deletions(-) 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 1c5c26e..46549fa 100644 --- a/go.sum +++ b/go.sum @@ -1,8 +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-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/schema/schemas b/schema/schemas index 849214b..3e9868b 160000 --- a/schema/schemas +++ b/schema/schemas @@ -1 +1 @@ -Subproject commit 849214bd225997e95f39c2a9a6aa10d83b44ac42 +Subproject commit 3e9868b02654aa64653bc8235c969d68aae8ee32