From 0cb5c8f8dc6b241e0433eeb3e552557ea725da56 Mon Sep 17 00:00:00 2001 From: dylan Date: Fri, 10 Nov 2023 15:31:24 -0800 Subject: [PATCH 1/8] feat: tools-2692 add metadata to config files --- asconf/metadata/metadata.go | 51 +++++++ asconf/metadata/metadata_test.go | 193 ++++++++++++++++++++++++ cmd/convert.go | 52 ++++++- cmd/utils.go | 71 +++++++++ integration_test.go | 67 +++++++- testdata/cases/metadata/conf-tests.json | 1 + testdata/cases/metadata/metadata.conf | 78 ++++++++++ testdata/cases/metadata/metadata.yaml | 58 +++++++ testdata/cases/metadata/versions.json | 1 + testdata/cases/metadata/yaml-tests.json | 1 + 10 files changed, 566 insertions(+), 7 deletions(-) create mode 100644 asconf/metadata/metadata.go create mode 100644 asconf/metadata/metadata_test.go create mode 100644 testdata/cases/metadata/conf-tests.json create mode 100644 testdata/cases/metadata/metadata.conf create mode 100644 testdata/cases/metadata/metadata.yaml create mode 100644 testdata/cases/metadata/versions.json create mode 100644 testdata/cases/metadata/yaml-tests.json diff --git a/asconf/metadata/metadata.go b/asconf/metadata/metadata.go new file mode 100644 index 0000000..cf5690c --- /dev/null +++ b/asconf/metadata/metadata.go @@ -0,0 +1,51 @@ +package metadata + +import ( + "fmt" + "regexp" +) + +var commentChar string +var findComments *regexp.Regexp + +func init() { + commentChar = "#" + findComments = regexp.MustCompile(commentChar + `(?m)\s*(.+):\s*(.+)\s*$`) +} + +// type Data struct { +// AerospikeVersion string `aero-meta:"aerospike-server-version"` +// AsadmVersion string `aero-meta:"asadm-version"` +// AsconfigVersion string `aero-meta:"asconfig-version"` +// } + +func UnmarshalText(src []byte, dst map[string]string) error { + matches := findComments.FindAllSubmatch(src, -1) + + for _, match := range matches { + // 0 index is entire line + k := match[1] + v := match[2] + // only save the first occurrence of k + if _, ok := dst[string(k)]; !ok { + dst[string(k)] = string(v) + } + } + + return nil +} + +func formatLine(k string, v any) string { + fmtStr := "%s %s: %v" + return fmt.Sprintf(fmtStr, commentChar, k, v) +} + +func MarshalText(src map[string]string) ([]byte, error) { + res := []byte{} + + for k, v := range src { + res = append(res, []byte(formatLine(k, v)+"\n")...) + } + + return res, nil +} diff --git a/asconf/metadata/metadata_test.go b/asconf/metadata/metadata_test.go new file mode 100644 index 0000000..d0be475 --- /dev/null +++ b/asconf/metadata/metadata_test.go @@ -0,0 +1,193 @@ +//go:build unit +// +build unit + +package metadata_test + +import ( + "reflect" + "testing" + + "github.com/aerospike/asconfig/asconf/metadata" +) + +var testBasic = ` +# comment about metadata +# a: b +other data +` + +var testConf = ` +# comment about metadata +# aerospike-server-version: 6.4.0.1 +# asconfig-version: 0.12.0 +# asadm-version: 2.20.0 + +# + +logging { + + file /dummy/file/path2 { + context any info # aerospike-server-version: collide + } +} +` + +var testConfNoMeta = ` +namespace ns2 { + replication-factor 2 + memory-size 8G + index-type shmem # comment mid config + sindex-type shmem + storage-engine memory +} +# comment +` + +var testConfPartialMeta = ` +namespace ns1 { + replication-factor 2 + memory-size 4G + + index-type flash { + mount /dummy/mount/point1 /test/mount2 + mounts-high-water-pct 30 + mounts-size-limit 10G + } + + # comment about metadata + # aerospike-server-version: 6.4.0.1 + # other-item: a long value +` + +func TestUnmarshal(t *testing.T) { + type args struct { + src []byte + dst map[string]string + } + tests := []struct { + name string + args args + want map[string]string + wantErr bool + }{ + { + name: "t1", + args: args{ + src: []byte(testConf), + dst: map[string]string{}, + }, + want: map[string]string{ + "aerospike-server-version": "6.4.0.1", + "asadm-version": "2.20.0", + "asconfig-version": "0.12.0", + }, + wantErr: false, + }, + { + name: "t2", + args: args{ + src: []byte(testConfNoMeta), + dst: map[string]string{}, + }, + want: map[string]string{}, + wantErr: false, + }, + { + name: "t3", + args: args{ + src: []byte(testConfPartialMeta), + dst: map[string]string{}, + }, + want: map[string]string{ + "aerospike-server-version": "6.4.0.1", + "other-item": "a long value", + }, + wantErr: false, + }, + { + name: "t4", + args: args{ + src: []byte(testBasic), + dst: map[string]string{}, + }, + want: map[string]string{ + "a": "b", + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := metadata.UnmarshalText(tt.args.src, tt.args.dst); (err != nil) != tt.wantErr { + t.Errorf("Unmarshal() error = %v, wantErr %v", err, tt.wantErr) + } + if !reflect.DeepEqual(tt.args.dst, tt.want) { + t.Errorf("MarshalText() = %v, want %v", tt.args.dst, tt.want) + } + }) + } +} + +var testMarshalMetaComplete = `# aerospike-server-version: 7.0.0.0 +# asadm-version: 2.20.0 +# asconfig-version: 0.12.0 +` + +var testMarshalMetaNone = "" + +var testMarshalMetaPartial = `# aerospike-server-version: 6.4.0 +` + +func TestMarshalText(t *testing.T) { + type args struct { + src map[string]string + } + tests := []struct { + name string + args args + want []byte + wantErr bool + }{ + { + name: "t1", + args: args{ + src: map[string]string{ + "aerospike-server-version": "7.0.0.0", + "asadm-version": "2.20.0", + "asconfig-version": "0.12.0", + }, + }, + want: []byte(testMarshalMetaComplete), + wantErr: false, + }, + { + name: "t2", + args: args{ + src: map[string]string{}, + }, + want: []byte(testMarshalMetaNone), + wantErr: false, + }, + { + name: "t3", + args: args{ + src: map[string]string{ + "aerospike-server-version": "6.4.0", + }, + }, + want: []byte(testMarshalMetaPartial), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := metadata.MarshalText(tt.args.src) + if (err != nil) != tt.wantErr { + t.Errorf("MarshalText() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("MarshalText() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/cmd/convert.go b/cmd/convert.go index df868bb..26f70ab 100644 --- a/cmd/convert.go +++ b/cmd/convert.go @@ -9,6 +9,7 @@ import ( "github.com/aerospike/aerospike-management-lib/asconfig" "github.com/aerospike/asconfig/asconf" + "github.com/aerospike/asconfig/asconf/metadata" "github.com/spf13/cobra" ) @@ -39,7 +40,9 @@ func newConvertCmd() *cobra.Command { 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. - Specifying the server version that will use the aerospike.conf is required. + Specifying the Aerospike Database version that will use the aerospike.conf is required unless + version metadata is present in the file or the --force option is used. + Aerospike Database metadata is written to files generated by asconfig. Usage examples... convert local file "aerospike.yaml" to aerospike config format for version 6.2.0 and write it to local file "aerospike.conf." @@ -51,7 +54,10 @@ func newConvertCmd() *cobra.Command { Normally the file format is inferred from file extensions ".yaml" ".conf" etc. Source format can be forced with the --format flag. Ex: asconfig convert -a "6.2.0" --format yaml example_file - If a file path is not provided, asconfig reads the file contents from stdin.`, + If a file path is not provided, asconfig reads the file contents from stdin. + Ex: asconfig convert -a "6.4.0" + If the file has been converted by asconfig before, the --aerospike-version option is not needed. + Ex: asconfig convert -a "6.4.0" aerospike.yaml | asconfig convert --format conf`, RunE: func(cmd *cobra.Command, args []string) error { logger.Debug("Running convert command") @@ -99,6 +105,15 @@ func newConvertCmd() *cobra.Command { return fmt.Errorf("%w: %s", errInvalidFormat, srcFormat) } + // if the version option is empty, + // try populating from the metadata + if version == "" { + version, err = getMetaDataItem(fdata, metaKeyAerospikeVersion) + if err != nil { + return err + } + } + conf, err := asconf.NewAsconf( fdata, srcFormat, @@ -124,6 +139,17 @@ func newConvertCmd() *cobra.Command { return err } + // prepend metadata to the config output + mtext, err := genMetaDataText(metaDataArgs{ + src: fdata, + aerospikeVersion: version, + asconfigVersion: VERSION, + }) + if err != nil { + return err + } + out = append(mtext, out...) + outputPath, err := cmd.Flags().GetString("output") if err != nil { return err @@ -188,8 +214,22 @@ func newConvertCmd() *cobra.Command { return err } - if !force { - cmd.MarkFlagRequired("aerospike-version") + cfgData, err := os.ReadFile(args[0]) + if err != nil { + return err + } + + metaData := map[string]string{} + metadata.UnmarshalText(cfgData, metaData) + + // if the aerospike server version is in the cfg file's + // metadata, don't mark --aerospike-version as required + var aeroVersionRequired bool + if _, ok := metaData[metaKeyAerospikeVersion]; !ok { + if !force { + cmd.MarkFlagRequired("aerospike-version") + aeroVersionRequired = true + } } av, err := cmd.Flags().GetString("aerospike-version") @@ -197,7 +237,7 @@ func newConvertCmd() *cobra.Command { return err } - if !force { + if aeroVersionRequired { if av == "" { return errors.Join(errMissingAerospikeVersion, err) } @@ -220,7 +260,7 @@ 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") + 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 or the configuration file being converted contains Aerospike Database version metadata.") 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..cfc9825 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -2,14 +2,85 @@ package cmd import ( "errors" + "fmt" "path/filepath" "strings" "github.com/aerospike/asconfig/asconf" + "github.com/aerospike/asconfig/asconf/metadata" "github.com/spf13/cobra" ) +const ( + metaKeyAerospikeVersion = "aerospike-server-version" + metaKeyAsconfigVersion = "asconfig-version" + metaKeyAsadmVersion = "asadm-version" +) + +var metaDataKeys = map[string]struct{}{ + metaKeyAerospikeVersion: struct{}{}, + metaKeyAsconfigVersion: struct{}{}, + metaKeyAsadmVersion: struct{}{}, +} + +type metaDataArgs struct { + src []byte + aerospikeVersion string + asconfigVersion string +} + +func genMetaDataText(args metaDataArgs) ([]byte, error) { + + metaHeader := "# *** Aerospike Metadata Generated by Asconfig ***" + + src := args.src + aev := args.aerospikeVersion + asv := args.asconfigVersion + + mdata := map[string]string{} + err := metadata.UnmarshalText(src, mdata) + if err != nil { + return nil, err + } + + mdata[metaKeyAerospikeVersion] = aev + mdata[metaKeyAsconfigVersion] = asv + + filteredMdata := map[string]string{} + for k, v := range mdata { + if _, ok := metaDataKeys[k]; ok { + filteredMdata[k] = v + } + } + + mtext, err := metadata.MarshalText(filteredMdata) + if err != nil { + return nil, err + } + + metaFooter := "# *** End Aerospike Metadata ***" + + mtext = []byte(fmt.Sprintf("%s\n%s%s\n", metaHeader, mtext, metaFooter)) + + return mtext, nil +} + +func getMetaDataItem(src []byte, key string) (string, error) { + mdata := map[string]string{} + err := metadata.UnmarshalText(src, mdata) + if err != nil { + return "", err + } + + val, ok := mdata[key] + if !ok { + return "", fmt.Errorf("metadata does not contain %s", key) + } + + return val, nil +} + // 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/integration_test.go b/integration_test.go index abaca4a..40792cd 100644 --- a/integration_test.go +++ b/integration_test.go @@ -13,12 +13,15 @@ import ( "os" "os/exec" "path/filepath" + "reflect" "regexp" "strconv" "strings" "testing" "time" + "github.com/aerospike/asconfig/asconf/metadata" + "github.com/aerospike/asconfig/cmd" "github.com/aerospike/asconfig/testutils" "github.com/docker/docker/api/types" @@ -797,7 +800,7 @@ func diff(args ...string) ([]byte, error) { args = append([]string{"diff"}, args...) com := exec.Command(binPath+"/asconfig.test", args...) com.Env = []string{"GOCOVERDIR=" + coveragePath} - diff, err := com.Output() + diff, err := com.CombinedOutput() if err != nil { err = fmt.Errorf("diff failed err: %s, out: %s", err, string(diff)) } @@ -916,3 +919,65 @@ func TestConvertStdin(t *testing.T) { } } + +type convertMetaDataTest struct { + expectedMetaData map[string]string + expectedFile string + arguments []string +} + +var convertMetaDataTests = []convertMetaDataTest{ + { + expectedMetaData: map[string]string{ + "aerospike-server-version": "6.2.0.2", + "asconfig-version": cmd.VERSION, + }, + expectedFile: filepath.Join(expectedPath, "all_flash_cluster_cr.conf"), + arguments: []string{"convert", "-a", "6.2.0.2", filepath.Join(sourcePath, "all_flash_cluster_cr.yaml")}, + }, + { + expectedMetaData: map[string]string{ + "aerospike-server-version": "6.4.0", + "asconfig-version": cmd.VERSION, + "asadm-version": "2.12.0", + }, + expectedFile: filepath.Join(extraTestPath, "metadata", "metadata.yaml"), + arguments: []string{"convert", filepath.Join(extraTestPath, "metadata", "metadata.conf")}, + }, +} + +func TestConvertMetaData(t *testing.T) { + for _, tf := range convertMetaDataTests { + tmpOutFileName := filepath.Join(destinationPath, "stdinConvertTmp") + + tf.arguments = append(tf.arguments, "-o", tmpOutFileName) + com := exec.Command(binPath+"/asconfig.test", tf.arguments...) + com.Env = []string{"GOCOVERDIR=" + coveragePath} + output, err := com.CombinedOutput() + if err != nil { + t.Errorf("convert failed err: %s, out: %s", err, string(output)) + } + + fileOut, err := os.ReadFile(tmpOutFileName) + if err != nil { + t.Error(err) + } + + metaData := map[string]string{} + err = metadata.UnmarshalText(fileOut, metaData) + if err != nil { + t.Errorf("metadata unmarshal err: %s, out: %s", err, string(fileOut)) + } + + if !reflect.DeepEqual(metaData, tf.expectedMetaData) { + t.Errorf("METADATA NOT EQUAL\nCASE: %+v\nACTUAL: %+v\nEXPECTED: %+v\n", tf, metaData, tf.expectedMetaData) + } + + diffFormat := filepath.Ext(tf.expectedFile) + diffFormat = strings.TrimPrefix(diffFormat, ".") + + if _, err := diff(tmpOutFileName, tf.expectedFile, "--format", diffFormat); err != nil { + t.Errorf("\nTESTCASE: %+v\nERR: %+v\n", tf, err) + } + } +} diff --git a/testdata/cases/metadata/conf-tests.json b/testdata/cases/metadata/conf-tests.json new file mode 100644 index 0000000..933b97b --- /dev/null +++ b/testdata/cases/metadata/conf-tests.json @@ -0,0 +1 @@ +[{"Source":"testdata/cases/metadata/metadata.conf","Destination":"testdata/cases/metadata/metadata-res-.yaml","Expected":"testdata/cases/metadata/metadata.yaml","Arguments":["convert","--aerospike-version","6.4.0","--format","asconfig","--output","testdata/cases/metadata/metadata-res-.yaml"],"SkipServerTest":false,"ServerErrorAllowList":null,"ServerImage":"","DockerAuth":{"Username":"","Password":""}}] \ No newline at end of file diff --git a/testdata/cases/metadata/metadata.conf b/testdata/cases/metadata/metadata.conf new file mode 100644 index 0000000..fd0a2cc --- /dev/null +++ b/testdata/cases/metadata/metadata.conf @@ -0,0 +1,78 @@ +# *** Aerospike Metadata Generated by Asconfig *** +# aerospike-server-version: 6.4.0 +# asconfig-version: 0.12.0 +# asadm-version: 2.12.0 +# garbage-value: hi +# *** End Aerospike Metadata *** +service { + user root + 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 +} + +logging { + + file /dummy/file/path2 { + context any info + } +} + +network { + service { + address any + port 3000 + } + + heartbeat { + mode multicast + multicast-group 127.0.0.1 + port 9918 + + + + + interval 150 + timeout 10 + } + + fabric { + port 3001 + } + + info { + port 3003 + } +} + +namespace ns1 { + replication-factor 2 + memory-size 4G + + index-type flash { + mount /dummy/mount/point1 /test/mount2 + mounts-high-water-pct 30 + mounts-size-limit 10G + } + + sindex-type flash { + mount /dummy/mount/point3 + mounts-high-water-pct 60 + mounts-size-limit 20000M + } + + storage-engine memory +} + +namespace ns2 { + replication-factor 2 + memory-size 8G + index-type shmem + sindex-type shmem + storage-engine memory +} + diff --git a/testdata/cases/metadata/metadata.yaml b/testdata/cases/metadata/metadata.yaml new file mode 100644 index 0000000..17ae934 --- /dev/null +++ b/testdata/cases/metadata/metadata.yaml @@ -0,0 +1,58 @@ +# *** Aerospike Metadata Generated by Asconfig *** +# aerospike-server-version: 6.4.0 +# asconfig-version: 0.12.0 +# *** End Aerospike Metadata *** +logging: + - any: info + name: /dummy/file/path2 +namespaces: + - index-type: + type: shmem + memory-size: 8589934592 + name: ns2 + replication-factor: 2 + sindex-type: + type: shmem + storage-engine: + type: memory + - index-type: + mounts: + - /dummy/mount/point1 /test/mount2 + mounts-high-water-pct: 30 + mounts-size-limit: 10737418240 + type: flash + memory-size: 4294967296 + name: ns1 + replication-factor: 2 + sindex-type: + mounts: + - /dummy/mount/point3 + mounts-high-water-pct: 60 + mounts-size-limit: 20971520000 + type: flash + storage-engine: + type: memory +network: + fabric: + port: 3001 + heartbeat: + interval: 150 + mode: multicast + multicast-groups: + - 127.0.0.1 + port: 9918 + timeout: 10 + info: + port: 3003 + service: + addresses: + - any + port: 3000 +service: + 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 diff --git a/testdata/cases/metadata/versions.json b/testdata/cases/metadata/versions.json new file mode 100644 index 0000000..cdc3312 --- /dev/null +++ b/testdata/cases/metadata/versions.json @@ -0,0 +1 @@ +{"TestedVersion":"6.4.0","OriginallyUsedVersion":"6.2.0.2"} \ No newline at end of file diff --git a/testdata/cases/metadata/yaml-tests.json b/testdata/cases/metadata/yaml-tests.json new file mode 100644 index 0000000..f0a39e0 --- /dev/null +++ b/testdata/cases/metadata/yaml-tests.json @@ -0,0 +1 @@ +[{"Source":"testdata/cases/metadata/metadata.yaml","Destination":"testdata/cases/metadata/metadata-res-.conf","Expected":"testdata/cases/metadata/metadata.conf","Arguments":["convert","--aerospike-version","6.4.0","--format","yaml","--output","testdata/cases/metadata/metadata-res-.conf"],"SkipServerTest":false,"ServerErrorAllowList":null,"ServerImage":"","DockerAuth":{"Username":"","Password":""}}] \ No newline at end of file From 7a0da661ee8dfd813134f390b2fa34423132bb2d Mon Sep 17 00:00:00 2001 From: dylan Date: Fri, 10 Nov 2023 15:58:04 -0800 Subject: [PATCH 2/8] self review --- asconf/metadata/metadata.go | 10 ++-------- asconf/metadata/metadata_test.go | 10 +++++----- cmd/convert.go | 4 ++-- cmd/utils.go | 6 +++--- 4 files changed, 12 insertions(+), 18 deletions(-) diff --git a/asconf/metadata/metadata.go b/asconf/metadata/metadata.go index cf5690c..dc7360d 100644 --- a/asconf/metadata/metadata.go +++ b/asconf/metadata/metadata.go @@ -13,13 +13,7 @@ func init() { findComments = regexp.MustCompile(commentChar + `(?m)\s*(.+):\s*(.+)\s*$`) } -// type Data struct { -// AerospikeVersion string `aero-meta:"aerospike-server-version"` -// AsadmVersion string `aero-meta:"asadm-version"` -// AsconfigVersion string `aero-meta:"asconfig-version"` -// } - -func UnmarshalText(src []byte, dst map[string]string) error { +func Unmarshal(src []byte, dst map[string]string) error { matches := findComments.FindAllSubmatch(src, -1) for _, match := range matches { @@ -40,7 +34,7 @@ func formatLine(k string, v any) string { return fmt.Sprintf(fmtStr, commentChar, k, v) } -func MarshalText(src map[string]string) ([]byte, error) { +func Marshal(src map[string]string) ([]byte, error) { res := []byte{} for k, v := range src { diff --git a/asconf/metadata/metadata_test.go b/asconf/metadata/metadata_test.go index d0be475..8ceb001 100644 --- a/asconf/metadata/metadata_test.go +++ b/asconf/metadata/metadata_test.go @@ -118,11 +118,11 @@ func TestUnmarshal(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if err := metadata.UnmarshalText(tt.args.src, tt.args.dst); (err != nil) != tt.wantErr { + if err := metadata.Unmarshal(tt.args.src, tt.args.dst); (err != nil) != tt.wantErr { t.Errorf("Unmarshal() error = %v, wantErr %v", err, tt.wantErr) } if !reflect.DeepEqual(tt.args.dst, tt.want) { - t.Errorf("MarshalText() = %v, want %v", tt.args.dst, tt.want) + t.Errorf("Unmarshal() = %v, want %v", tt.args.dst, tt.want) } }) } @@ -180,13 +180,13 @@ func TestMarshalText(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := metadata.MarshalText(tt.args.src) + got, err := metadata.Marshal(tt.args.src) if (err != nil) != tt.wantErr { - t.Errorf("MarshalText() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("Marshal() error = %v, wantErr %v", err, tt.wantErr) return } if !reflect.DeepEqual(got, tt.want) { - t.Errorf("MarshalText() = %v, want %v", got, tt.want) + t.Errorf("Marshal() = %v, want %v", got, tt.want) } }) } diff --git a/cmd/convert.go b/cmd/convert.go index 26f70ab..b8e3c81 100644 --- a/cmd/convert.go +++ b/cmd/convert.go @@ -110,7 +110,7 @@ func newConvertCmd() *cobra.Command { if version == "" { version, err = getMetaDataItem(fdata, metaKeyAerospikeVersion) if err != nil { - return err + return errors.Join(errMissingAerospikeVersion, err) } } @@ -220,7 +220,7 @@ func newConvertCmd() *cobra.Command { } metaData := map[string]string{} - metadata.UnmarshalText(cfgData, metaData) + metadata.Unmarshal(cfgData, metaData) // if the aerospike server version is in the cfg file's // metadata, don't mark --aerospike-version as required diff --git a/cmd/utils.go b/cmd/utils.go index cfc9825..d6429c8 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -39,7 +39,7 @@ func genMetaDataText(args metaDataArgs) ([]byte, error) { asv := args.asconfigVersion mdata := map[string]string{} - err := metadata.UnmarshalText(src, mdata) + err := metadata.Unmarshal(src, mdata) if err != nil { return nil, err } @@ -54,7 +54,7 @@ func genMetaDataText(args metaDataArgs) ([]byte, error) { } } - mtext, err := metadata.MarshalText(filteredMdata) + mtext, err := metadata.Marshal(filteredMdata) if err != nil { return nil, err } @@ -68,7 +68,7 @@ func genMetaDataText(args metaDataArgs) ([]byte, error) { func getMetaDataItem(src []byte, key string) (string, error) { mdata := map[string]string{} - err := metadata.UnmarshalText(src, mdata) + err := metadata.Unmarshal(src, mdata) if err != nil { return "", err } From 1557de2a24db5b2cd366cf4e677234468d8c2e64 Mon Sep 17 00:00:00 2001 From: dylan Date: Fri, 10 Nov 2023 16:01:05 -0800 Subject: [PATCH 3/8] self review pt2 --- integration_test.go | 4 ++-- testdata/cases/metadata/metadata.conf | 2 +- testdata/cases/metadata/metadata.yaml | 3 ++- testdata/cases/metadata/versions.json | 2 +- testdata/cases/metadata/yaml-tests.json | 2 +- 5 files changed, 7 insertions(+), 6 deletions(-) diff --git a/integration_test.go b/integration_test.go index 40792cd..e8cb148 100644 --- a/integration_test.go +++ b/integration_test.go @@ -937,7 +937,7 @@ var convertMetaDataTests = []convertMetaDataTest{ }, { expectedMetaData: map[string]string{ - "aerospike-server-version": "6.4.0", + "aerospike-server-version": "6.4.0.1", "asconfig-version": cmd.VERSION, "asadm-version": "2.12.0", }, @@ -964,7 +964,7 @@ func TestConvertMetaData(t *testing.T) { } metaData := map[string]string{} - err = metadata.UnmarshalText(fileOut, metaData) + err = metadata.Unmarshal(fileOut, metaData) if err != nil { t.Errorf("metadata unmarshal err: %s, out: %s", err, string(fileOut)) } diff --git a/testdata/cases/metadata/metadata.conf b/testdata/cases/metadata/metadata.conf index fd0a2cc..9f4409d 100644 --- a/testdata/cases/metadata/metadata.conf +++ b/testdata/cases/metadata/metadata.conf @@ -1,5 +1,5 @@ # *** Aerospike Metadata Generated by Asconfig *** -# aerospike-server-version: 6.4.0 +# aerospike-server-version: 6.4.0.1 # asconfig-version: 0.12.0 # asadm-version: 2.12.0 # garbage-value: hi diff --git a/testdata/cases/metadata/metadata.yaml b/testdata/cases/metadata/metadata.yaml index 17ae934..7c5bf29 100644 --- a/testdata/cases/metadata/metadata.yaml +++ b/testdata/cases/metadata/metadata.yaml @@ -1,6 +1,7 @@ # *** Aerospike Metadata Generated by Asconfig *** -# aerospike-server-version: 6.4.0 +# aerospike-server-version: 6.4.0.1 # asconfig-version: 0.12.0 +# asadm-version: 2.12.0 # *** End Aerospike Metadata *** logging: - any: info diff --git a/testdata/cases/metadata/versions.json b/testdata/cases/metadata/versions.json index cdc3312..e2a14fd 100644 --- a/testdata/cases/metadata/versions.json +++ b/testdata/cases/metadata/versions.json @@ -1 +1 @@ -{"TestedVersion":"6.4.0","OriginallyUsedVersion":"6.2.0.2"} \ No newline at end of file +{"TestedVersion":"6.4.0.1","OriginallyUsedVersion":"6.4.0.1"} \ No newline at end of file diff --git a/testdata/cases/metadata/yaml-tests.json b/testdata/cases/metadata/yaml-tests.json index f0a39e0..78698a9 100644 --- a/testdata/cases/metadata/yaml-tests.json +++ b/testdata/cases/metadata/yaml-tests.json @@ -1 +1 @@ -[{"Source":"testdata/cases/metadata/metadata.yaml","Destination":"testdata/cases/metadata/metadata-res-.conf","Expected":"testdata/cases/metadata/metadata.conf","Arguments":["convert","--aerospike-version","6.4.0","--format","yaml","--output","testdata/cases/metadata/metadata-res-.conf"],"SkipServerTest":false,"ServerErrorAllowList":null,"ServerImage":"","DockerAuth":{"Username":"","Password":""}}] \ No newline at end of file +[{"Source":"testdata/cases/metadata/metadata.yaml","Destination":"testdata/cases/metadata/metadata-res-.conf","Expected":"testdata/cases/metadata/metadata.conf","Arguments":["convert","--aerospike-version","6.4.0.1","--format","yaml","--output","testdata/cases/metadata/metadata-res-.conf"],"SkipServerTest":false,"ServerErrorAllowList":null,"ServerImage":"","DockerAuth":{"Username":"","Password":""}}] \ No newline at end of file From b625633201e9c766a391531e6ad1480d124aba94 Mon Sep 17 00:00:00 2001 From: dylan Date: Fri, 10 Nov 2023 16:19:46 -0800 Subject: [PATCH 4/8] fix aerospike server version --- testdata/cases/metadata/conf-tests.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testdata/cases/metadata/conf-tests.json b/testdata/cases/metadata/conf-tests.json index 933b97b..6d4dd5e 100644 --- a/testdata/cases/metadata/conf-tests.json +++ b/testdata/cases/metadata/conf-tests.json @@ -1 +1 @@ -[{"Source":"testdata/cases/metadata/metadata.conf","Destination":"testdata/cases/metadata/metadata-res-.yaml","Expected":"testdata/cases/metadata/metadata.yaml","Arguments":["convert","--aerospike-version","6.4.0","--format","asconfig","--output","testdata/cases/metadata/metadata-res-.yaml"],"SkipServerTest":false,"ServerErrorAllowList":null,"ServerImage":"","DockerAuth":{"Username":"","Password":""}}] \ No newline at end of file +[{"Source":"testdata/cases/metadata/metadata.conf","Destination":"testdata/cases/metadata/metadata-res-.yaml","Expected":"testdata/cases/metadata/metadata.yaml","Arguments":["convert","--aerospike-version","6.4.0.1","--format","asconfig","--output","testdata/cases/metadata/metadata-res-.yaml"],"SkipServerTest":false,"ServerErrorAllowList":null,"ServerImage":"","DockerAuth":{"Username":"","Password":""}}] \ No newline at end of file From b12b71a983537d78e92cbb2c4ce17029fc10b34a Mon Sep 17 00:00:00 2001 From: dylan Date: Fri, 10 Nov 2023 16:38:51 -0800 Subject: [PATCH 5/8] sort metadata text --- asconf/metadata/metadata.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/asconf/metadata/metadata.go b/asconf/metadata/metadata.go index dc7360d..e484f08 100644 --- a/asconf/metadata/metadata.go +++ b/asconf/metadata/metadata.go @@ -3,6 +3,7 @@ package metadata import ( "fmt" "regexp" + "sort" ) var commentChar string @@ -36,9 +37,16 @@ func formatLine(k string, v any) string { func Marshal(src map[string]string) ([]byte, error) { res := []byte{} + lines := make([]string, len(src)) for k, v := range src { - res = append(res, []byte(formatLine(k, v)+"\n")...) + lines = append(lines, formatLine(k, v)+"\n") + } + + sort.Strings(lines) + + for _, v := range lines { + res = append(res, []byte(v)...) } return res, nil From 54d502793b4c7d4872fcba169d62ef23e6943e6b Mon Sep 17 00:00:00 2001 From: dylan Date: Tue, 14 Nov 2023 10:16:39 -0800 Subject: [PATCH 6/8] review changes pt1 --- asconf/metadata/metadata.go | 7 +++++++ cmd/utils.go | 15 +-------------- testdata/cases/metadata/metadata.conf | 1 - 3 files changed, 8 insertions(+), 15 deletions(-) diff --git a/asconf/metadata/metadata.go b/asconf/metadata/metadata.go index e484f08..0b6ef1c 100644 --- a/asconf/metadata/metadata.go +++ b/asconf/metadata/metadata.go @@ -11,6 +11,13 @@ var findComments *regexp.Regexp func init() { commentChar = "#" + // findComments matches text of the form ` : ` + // for example, parsing... + // # comment about metadata + // # a: b + // other data + // matches + // # a: b findComments = regexp.MustCompile(commentChar + `(?m)\s*(.+):\s*(.+)\s*$`) } diff --git a/cmd/utils.go b/cmd/utils.go index d6429c8..84e2367 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -18,12 +18,6 @@ const ( metaKeyAsadmVersion = "asadm-version" ) -var metaDataKeys = map[string]struct{}{ - metaKeyAerospikeVersion: struct{}{}, - metaKeyAsconfigVersion: struct{}{}, - metaKeyAsadmVersion: struct{}{}, -} - type metaDataArgs struct { src []byte aerospikeVersion string @@ -47,14 +41,7 @@ func genMetaDataText(args metaDataArgs) ([]byte, error) { mdata[metaKeyAerospikeVersion] = aev mdata[metaKeyAsconfigVersion] = asv - filteredMdata := map[string]string{} - for k, v := range mdata { - if _, ok := metaDataKeys[k]; ok { - filteredMdata[k] = v - } - } - - mtext, err := metadata.Marshal(filteredMdata) + mtext, err := metadata.Marshal(mdata) if err != nil { return nil, err } diff --git a/testdata/cases/metadata/metadata.conf b/testdata/cases/metadata/metadata.conf index 9f4409d..839525c 100644 --- a/testdata/cases/metadata/metadata.conf +++ b/testdata/cases/metadata/metadata.conf @@ -2,7 +2,6 @@ # aerospike-server-version: 6.4.0.1 # asconfig-version: 0.12.0 # asadm-version: 2.12.0 -# garbage-value: hi # *** End Aerospike Metadata *** service { user root From 277e80efdd1f1d053fd94f62519fe92aef2d7398 Mon Sep 17 00:00:00 2001 From: dylan Date: Tue, 14 Nov 2023 15:16:29 -0800 Subject: [PATCH 7/8] add metadata support to validate --- cmd/utils.go | 16 +++++++++++++--- cmd/validate.go | 30 ++++++++++++++++++++++++------ integration_test.go | 20 +++++++++++++------- 3 files changed, 50 insertions(+), 16 deletions(-) diff --git a/cmd/utils.go b/cmd/utils.go index cb7521c..22286e7 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -54,15 +54,25 @@ func genMetaDataText(args metaDataArgs) ([]byte, error) { return mtext, nil } -func getMetaDataItem(src []byte, key string) (string, error) { +func getMetaDataItemOptional(src []byte, key string) (string, error) { mdata := map[string]string{} err := metadata.Unmarshal(src, mdata) if err != nil { return "", err } - val, ok := mdata[key] - if !ok { + val := mdata[key] + + return val, nil +} + +func getMetaDataItem(src []byte, key string) (string, error) { + val, err := getMetaDataItemOptional(src, key) + if err != nil { + return "", err + } + + if val == "" { return "", fmt.Errorf("metadata does not contain %s", key) } diff --git a/cmd/validate.go b/cmd/validate.go index b193d47..c284f9b 100644 --- a/cmd/validate.go +++ b/cmd/validate.go @@ -48,25 +48,42 @@ func newValidateCmd() *cobra.Command { srcPath = args[0] } - version, err := cmd.Flags().GetString("aerospike-version") + srcFormat, err := getConfFileFormat(srcPath, cmd) if err != nil { return err } - logger.Debugf("Processing flag aerospike-version value=%s", version) + logger.Debugf("Processing flag format value=%v", srcFormat) - srcFormat, err := getConfFileFormat(srcPath, cmd) + fdata, err := os.ReadFile(srcPath) if err != nil { return err } - logger.Debugf("Processing flag format value=%v", srcFormat) + version, err := getMetaDataItemOptional(fdata, metaKeyAerospikeVersion) + if err != nil { + return errors.Join(errMissingAerospikeVersion, err) + } - fdata, err := os.ReadFile(srcPath) + // if the Aerospike server version was not in the file + // metadata, require that it is passed as an argument + if version == "" { + cmd.MarkFlagRequired("aerospike-version") + } + + versionArg, err := cmd.Flags().GetString("aerospike-version") if err != nil { return err } + // the command line --aerospike-version option overrides + // the metadata server version + if versionArg != "" { + version = versionArg + } + + logger.Debugf("Processing flag aerospike-version value=%s", version) + conf, err := asconf.NewAsconf( fdata, srcFormat, @@ -98,9 +115,10 @@ func newValidateCmd() *cobra.Command { } // flags and configuration settings + // --aerospike-version is required unless the server version + // is in the input config file's metadata commonFlags := getCommonFlags() res.Flags().AddFlagSet(commonFlags) - res.MarkFlagRequired("aerospike-version") res.Version = VERSION diff --git a/integration_test.go b/integration_test.go index c141da3..bf3133b 100644 --- a/integration_test.go +++ b/integration_test.go @@ -70,11 +70,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() @@ -997,9 +997,15 @@ var validateTests = []validateTest{ source: filepath.Join(sourcePath, "pmem_cluster_cr.yaml"), }, { - arguments: []string{"validate", "-a", "7.0.0", filepath.Join(extraTestPath, "server64/server64.yaml")}, + arguments: []string{"validate", filepath.Join(extraTestPath, "metadata", "metadata.conf")}, + expectError: false, + expectedResult: "", + source: filepath.Join(extraTestPath, "metadata", "metadata.conf"), + }, + { + arguments: []string{"validate", "-a", "7.0.0", filepath.Join(extraTestPath, "server64", "server64.yaml")}, expectError: true, - source: filepath.Join(extraTestPath, "server64/server64.yaml"), + 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 From bdca4ae6c887a77ab1d867f5145dacc2365f6f5a Mon Sep 17 00:00:00 2001 From: dylan Date: Tue, 14 Nov 2023 16:05:15 -0800 Subject: [PATCH 8/8] add back feature key --- integration_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/integration_test.go b/integration_test.go index bf3133b..096267a 100644 --- a/integration_test.go +++ b/integration_test.go @@ -70,11 +70,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()