From 5a00512b7ada89fc20f11cbbb0fc787c93320fea Mon Sep 17 00:00:00 2001 From: Thijs Schreijer Date: Fri, 31 Mar 2023 10:01:40 +0200 Subject: [PATCH] feat(merge): add a merge command (#13) the cli-command "merge" merges multiple decK files into one. --- .gitignore | 1 + .vscode/launch.json | 20 ++++ cmd/merge.go | 66 +++++++++++ cmd/openapi2kong.go | 4 +- deckformat/deckformat.go | 134 ++++++++++++++++++++++ filebasics/filebasics.go | 55 +++++++-- jsonbasics/jsonbasics.go | 6 +- merge/merge.go | 111 ++++++++++++++++++ merge/merge_test.go | 66 +++++++++++ merge/merge_testfiles/badversion.yml | 3 + merge/merge_testfiles/file1.yml | 30 +++++ merge/merge_testfiles/file2.yml | 15 +++ merge/merge_testfiles/file3.yml | 17 +++ merge/merge_testfiles/test1_expected.json | 31 +++++ merge/merge_testfiles/test2_expected.json | 31 +++++ merge/merge_testfiles/transform_false.yml | 3 + 16 files changed, 579 insertions(+), 14 deletions(-) create mode 100644 .vscode/launch.json create mode 100644 cmd/merge.go create mode 100644 deckformat/deckformat.go create mode 100644 merge/merge.go create mode 100644 merge/merge_test.go create mode 100644 merge/merge_testfiles/badversion.yml create mode 100644 merge/merge_testfiles/file1.yml create mode 100644 merge/merge_testfiles/file2.yml create mode 100644 merge/merge_testfiles/file3.yml create mode 100644 merge/merge_testfiles/test1_expected.json create mode 100644 merge/merge_testfiles/test2_expected.json create mode 100644 merge/merge_testfiles/transform_false.yml diff --git a/.gitignore b/.gitignore index f63b387..d5a5126 100644 --- a/.gitignore +++ b/.gitignore @@ -19,6 +19,7 @@ dist/ # generated test files convertoas3/oas3_testfiles/*.generated.json +merge/merge_testfiles/*_generated.json # the binary kced diff --git a/.vscode/launch.json b/.vscode/launch.json new file mode 100644 index 0000000..e0f504f --- /dev/null +++ b/.vscode/launch.json @@ -0,0 +1,20 @@ +{ + // Use IntelliSense to learn about possible attributes. + // Hover to view descriptions of existing attributes. + // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387 + "version": "0.2.0", + "configurations": [ + { + "name": "Launch Package", + "type": "go", + "request": "launch", + "mode": "auto", + "program": "main.go", + "args": ["merge", + "merge/merge_testfiles/file1.yml", + "merge/merge_testfiles/file2.yml", + "merge/merge_testfiles/file3.yml" + ] + } + ] +} diff --git a/cmd/merge.go b/cmd/merge.go new file mode 100644 index 0000000..7b1381e --- /dev/null +++ b/cmd/merge.go @@ -0,0 +1,66 @@ +/* +Copyright © 2023 NAME HERE +*/ +package cmd + +import ( + "fmt" + "log" + + "github.com/kong/go-apiops/filebasics" + "github.com/kong/go-apiops/merge" + "github.com/spf13/cobra" +) + +// Executes the CLI command "openapi2kong" +func executeMerge(cmd *cobra.Command, args []string) { + outputFilename, err := cmd.Flags().GetString("output-file") + if err != nil { + log.Fatalf(fmt.Sprintf("failed getting cli argument 'output-file'; %%w"), err) + } + + var asYaml bool + { + outputFormat, err := cmd.Flags().GetString("format") + if err != nil { + log.Fatalf(fmt.Sprintf("failed getting cli argument 'format'; %%w"), err) + } + if outputFormat == "yaml" { + asYaml = true + } else if outputFormat == "json" { + asYaml = false + } else { + log.Fatalf("expected '--format' to be either 'yaml' or 'json', got: '%s'", outputFormat) + } + } + + // do the work: read/merge + filebasics.MustWriteSerializedFile(outputFilename, merge.MustFiles(args), asYaml) +} + +// +// +// Define the CLI data for the openapi2kong command +// +// + +var mergeCmd = &cobra.Command{ + Use: "merge [flags] filename [...filename]", + Short: "Merges multiple decK files into one", + Long: `Merges multiple decK files into one. + +The files can be either json or yaml format. Will merge all top-level arrays by simply +concatenating them. Any other keys will be copied. The files will be processed in the order +provided. No checks on content will be done, eg. duplicates, nor any validations. + +If the input files are not compatible an error will be returned. Compatibility is +determined by the '_transform' and '_format_version' fields.`, + Run: executeMerge, + Args: cobra.MinimumNArgs(1), +} + +func init() { + rootCmd.AddCommand(mergeCmd) + mergeCmd.Flags().StringP("output-file", "o", "-", "output file to write. Use - to write to stdout") + mergeCmd.Flags().StringP("format", "", "yaml", "output format: json or yaml") +} diff --git a/cmd/openapi2kong.go b/cmd/openapi2kong.go index ada7ad2..6745e86 100644 --- a/cmd/openapi2kong.go +++ b/cmd/openapi2kong.go @@ -13,7 +13,7 @@ import ( ) // Executes the CLI command "openapi2kong" -func execute(cmd *cobra.Command, _ []string) { +func executeOpenapi2Kong(cmd *cobra.Command, _ []string) { inputFilename, err := cmd.Flags().GetString("state") if err != nil { log.Fatalf(fmt.Sprintf("failed getting cli argument 'spec'; %%w"), err) @@ -80,7 +80,7 @@ var openapi2kongCmd = &cobra.Command{ The example file has extensive annotations explaining the conversion process, as well as all supported custom annotations (x-kong-... directives). See: https://github.com/Kong/kced/blob/main/docs/learnservice_oas.yaml`, - Run: execute, + Run: executeOpenapi2Kong, } func init() { diff --git a/deckformat/deckformat.go b/deckformat/deckformat.go new file mode 100644 index 0000000..d548edf --- /dev/null +++ b/deckformat/deckformat.go @@ -0,0 +1,134 @@ +package deckformat + +import ( + "errors" + "fmt" + "strconv" + "strings" + + "github.com/kong/go-apiops/jsonbasics" +) + +const ( + VersionKey = "_format_version" + TransformKey = "_transform" +) + +// CompatibleTransform checks if 2 files are compatible, by '_transform' keys. +// Returns nil if compatible, and error otherwise. +func CompatibleTransform(data1 map[string]interface{}, data2 map[string]interface{}) error { + if data1 == nil { + panic("expected 'data1' to be non-nil") + } + if data2 == nil { + panic("expected 'data2' to be non-nil") + } + + transform1 := true // this is the default value + if data1[TransformKey] != nil { + var err error + if transform1, err = jsonbasics.GetBoolField(data1, TransformKey); err != nil { + return err + } + } + transform2 := true // this is the default value + if data2[TransformKey] != nil { + var err error + if transform2, err = jsonbasics.GetBoolField(data2, TransformKey); err != nil { + return err + } + } + + if transform1 != transform2 { + return errors.New("files with '_transform: true' (default) and '_transform: false' are not compatible") + } + + return nil +} + +// CompatibleVersion checks if 2 files are compatible, by '_format_version'. Version is compatible +// if they are the same major. Missing versions are assumed to be compatible. +// Returns nil if compatible, and error otherwise. +func CompatibleVersion(data1 map[string]interface{}, data2 map[string]interface{}) error { + if data1 == nil { + panic("expected 'data1' to be non-nil") + } + if data2 == nil { + panic("expected 'data2' to be non-nil") + } + + if data1[VersionKey] == nil { + if data2[VersionKey] == nil { + return nil // neither given , so assume compatible + } + // data1 omitted, just validate data2 has a proper version, any version will do + _, _, err := ParseFormatVersion(data2) + return err + } + + // data1 has a version + if data2[VersionKey] == nil { + // data2 omitted, just validate data1 has a proper version, any version will do + _, _, err := ParseFormatVersion(data1) + return err + } + + // both versions given, go parse them + major1, minor1, err1 := ParseFormatVersion(data1) + if err1 != nil { + return err1 + } + major2, minor2, err2 := ParseFormatVersion(data2) + if err2 != nil { + return err2 + } + + if major1 != major2 { + return fmt.Errorf("major versions are incompatible; %d.%d and %d.%d", major1, minor1, major2, minor2) + } + + return nil +} + +// CompatibleFile returns nil if the files are compatible. An error otherwise. +// see CompatibleVersion and CompatibleTransform for what compatibility means. +func CompatibleFile(data1 map[string]interface{}, data2 map[string]interface{}) error { + err := CompatibleTransform(data1, data2) + if err != nil { + return fmt.Errorf("files are incompatible; %w", err) + } + err = CompatibleVersion(data1, data2) + if err != nil { + return fmt.Errorf("files are incompatible; %w", err) + } + return nil +} + +// parseFormatVersion parses field `_format_version` and returns major+minor. +// Field must be present, a string, and have an 'x.y' format. Returns an error otherwise. +func ParseFormatVersion(data map[string]interface{}) (int, int, error) { + // get the file version and check it + v, err := jsonbasics.GetStringField(data, VersionKey) + if err != nil { + return 0, 0, errors.New("expected field '._format_version' to be a string in 'x.y' format") + } + elem := strings.Split(v, ".") + if len(elem) > 2 { + return 0, 0, errors.New("expected field '._format_version' to be a string in 'x.y' format") + } + + majorVersion, err := strconv.Atoi(elem[0]) + if err != nil { + return 0, 0, errors.New("expected field '._format_version' to be a string in 'x.y' format") + } + + minorVersion := 0 + if len(elem) > 1 { + minorVersion, err = strconv.Atoi(elem[1]) + if err != nil { + return 0, 0, errors.New("expected field '._format_version' to be a string in 'x.y' format") + } + } + + return majorVersion, minorVersion, nil +} diff --git a/filebasics/filebasics.go b/filebasics/filebasics.go index 565cb16..6c6541b 100644 --- a/filebasics/filebasics.go +++ b/filebasics/filebasics.go @@ -2,6 +2,7 @@ package filebasics import ( "encoding/json" + "errors" "fmt" "io" "log" @@ -14,9 +15,9 @@ const ( defaultJSONIndent = " " ) -// MustReadFile reads file contents. Will panic if reading fails. +// ReadFile reads file contents. // Reads from stdin if filename == "-" -func MustReadFile(filename string) *[]byte { +func ReadFile(filename string) (*[]byte, error) { var ( body []byte err error @@ -28,10 +29,21 @@ func MustReadFile(filename string) *[]byte { body, err = os.ReadFile(filename) } + if err != nil { + return nil, err + } + return &body, nil +} + +// MustReadFile reads file contents. Will panic if reading fails. +// Reads from stdin if filename == "-" +func MustReadFile(filename string) *[]byte { + body, err := ReadFile(filename) if err != nil { log.Fatalf("unable to read file: %v", err) } - return &body + + return body } // MustWriteFile writes the output to a file. Will panic if writing fails. @@ -80,26 +92,35 @@ func MustSerialize(content map[string]interface{}, asYaml bool) *[]byte { return &str } -// MustDeserialize will deserialize data as a JSON or YAML object. Will panic -// if deserializing fails or if it isn't an object. Will never return nil. -func MustDeserialize(data *[]byte) map[string]interface{} { +// Deserialize will deserialize data as a JSON or YAML object. Will return an error +// if deserializing fails or if it isn't an object. +func Deserialize(data *[]byte) (map[string]interface{}, error) { var output interface{} err1 := json.Unmarshal(*data, &output) if err1 != nil { err2 := yaml.Unmarshal(*data, &output) if err2 != nil { - log.Fatal("failed deserializing data as JSON (%w) and as YAML (%w)", err1, err2) + return nil, errors.New("failed deserializing data as JSON and as YAML") } } switch output := output.(type) { case map[string]interface{}: - return output + return output, nil } - log.Fatal("Expected the data to be an Object") - return nil // will never happen, unreachable. + return nil, errors.New("expected the data to be an Object") +} + +// MustDeserialize will deserialize data as a JSON or YAML object. Will panic +// if deserializing fails or if it isn't an object. Will never return nil. +func MustDeserialize(data *[]byte) map[string]interface{} { + jsondata, err := Deserialize(data) + if err != nil { + log.Fatal("%w", err) + } + return jsondata } // MustWriteSerializedFile will serialize the data and write it to a file. Will @@ -108,6 +129,20 @@ func MustWriteSerializedFile(filename string, content map[string]interface{}, as MustWriteFile(filename, MustSerialize(content, asYaml)) } +// DeserializeFile will read a JSON or YAML file and return the top-level object. Will return an +// error if it fails reading or the content isn't an object. Reads from stdin if filename == "-". +func DeserializeFile(filename string) (map[string]interface{}, error) { + bytedata, err := ReadFile(filename) + if err != nil { + return nil, err + } + data, err := Deserialize(bytedata) + if err != nil { + return nil, err + } + return data, nil +} + // MustDeserializeFile will read a JSON or YAML file and return the top-level object. Will // panic if it fails reading or the content isn't an object. Reads from stdin if filename == "-". // This will never return nil. diff --git a/jsonbasics/jsonbasics.go b/jsonbasics/jsonbasics.go index 92a9497..648c76d 100644 --- a/jsonbasics/jsonbasics.go +++ b/jsonbasics/jsonbasics.go @@ -42,9 +42,9 @@ func GetObjectArrayField(object map[string]interface{}, fieldName string) ([]map result := make([]map[string]interface{}, 0, len(arr)) j := 0 for _, expectedObject := range arr { - service, err := ToObject(expectedObject) + obj, err := ToObject(expectedObject) if err == nil { - result[j] = service + result[j] = obj j++ } } @@ -126,6 +126,8 @@ func GetStringIndex(arr []interface{}, index int) (string, error) { return "", fmt.Errorf("expected index '%d' to be a string, got %t", index, value) } +// GetBoolField returns a boolean from an object field. Returns an error if the field +// is not a boolean, or is not found. func GetBoolField(object map[string]interface{}, fieldName string) (bool, error) { value := object[fieldName] switch result := value.(type) { diff --git a/merge/merge.go b/merge/merge.go new file mode 100644 index 0000000..e22c00b --- /dev/null +++ b/merge/merge.go @@ -0,0 +1,111 @@ +package merge + +import ( + "fmt" + + "github.com/kong/go-apiops/deckformat" + "github.com/kong/go-apiops/filebasics" +) + +func merge2Files(data1 map[string]interface{}, data2 map[string]interface{}) map[string]interface{} { + mergedData := make(map[string]interface{}) + + for key, value := range data1 { + mergedData[key] = value + } + + for key, value := range data2 { + if existingValue, ok := mergedData[key]; ok { + // target already has this key + if a, ok := existingValue.([]interface{}); ok { + // we currently have an array + if b, ok := value.([]interface{}); ok { + // the new value also is an array, so append it + mergedData[key] = append(a, b...) + } else { + // the new value is not an array, overwrite the existing array + mergedData[key] = value + } + } else { + // existing value is not an array, overwrite it with the new value + mergedData[key] = value + } + } else { + // key doesn't exist in the target, so just insert + mergedData[key] = value + } + } + + return mergedData +} + +// MustFiles is identical to `Files` except that it will panic instead of return +// an error. +func MustFiles(filenames []string) map[string]interface{} { + result, err := Files(filenames) + if err != nil { + panic(err) + } + + return result +} + +// MergeFiles reads and merges files. Will merge all top-level arrays by simply +// concatenating them. Any other keys will be copied. The files will be processed +// in order provided. An error will be returned if files are incompatible. +// There are no checks on duplicates, etc... garbage-in-garbage-out. +func Files(filenames []string) (map[string]interface{}, error) { + if len(filenames) == 0 { + panic("no filenames provided") + } + + var result map[string]interface{} + minorVersion := 0 + + // traverse all files + for _, filename := range filenames { + // read the file + data, err := filebasics.DeserializeFile(filename) + if err != nil { + return nil, err + } + + if result == nil { + // set up initial map, ensure it is "compatible" with first entry + result = make(map[string]interface{}) + if data[deckformat.TransformKey] != nil { + result[deckformat.TransformKey] = data[deckformat.TransformKey] + } + if data[deckformat.VersionKey] != nil { + result[deckformat.VersionKey] = data[deckformat.VersionKey] + } + } + + // check compatibility + if err := deckformat.CompatibleFile(result, data); err != nil { + return nil, fmt.Errorf("failed to merge %s: %w", filename, err) + } + + // record minor version + _, m, _ := deckformat.ParseFormatVersion(data) + if m > minorVersion { + // we only track minor version, because majors must be the same to pass the + // compatibility check above + minorVersion = m + } + + result = merge2Files(result, data) + } + + // set final resulting format version + if result[deckformat.VersionKey] != nil { + ma, _, _ := deckformat.ParseFormatVersion(result) + if ma == 0 { + delete(result, deckformat.VersionKey) + } else { + result[deckformat.VersionKey] = fmt.Sprint(ma, ".", minorVersion) + } + } + + return result, nil +} diff --git a/merge/merge_test.go b/merge/merge_test.go new file mode 100644 index 0000000..4b765c6 --- /dev/null +++ b/merge/merge_test.go @@ -0,0 +1,66 @@ +package merge + +import ( + "fmt" + "strings" + "testing" + + "github.com/kong/go-apiops/filebasics" + "github.com/stretchr/testify/assert" +) + +func Test_Merge_Files_Good(t *testing.T) { + testSet := []struct { + files []string + expectedFilename string + }{ + {[]string{ + "./merge_testfiles/file1.yml", + "./merge_testfiles/file2.yml", + "./merge_testfiles/file3.yml", + }, "test1_expected.json"}, + {[]string{ + "./merge_testfiles/file3.yml", + "./merge_testfiles/file2.yml", + "./merge_testfiles/file1.yml", + }, "test2_expected.json"}, + } + + for _, tdata := range testSet { + res, err := Files(tdata.files) + if err != nil { + t.Error(fmt.Sprintf("'%s' didn't expect error: %%w", tdata.files), err) + } else { + expected := filebasics.MustReadFile("./merge_testfiles/" + tdata.expectedFilename) + result := filebasics.MustSerialize(res, false) + + filebasics.MustWriteSerializedFile("./merge_testfiles/"+ + strings.Replace(tdata.expectedFilename, "_expected.", "_generated.", -1), res, false) + assert.JSONEq(t, string(*expected), string(*result), + "'%s': the JSON blobs should be equal", tdata.files) + } + } +} + +func Test_Merge_Files_Bad(t *testing.T) { + testSet := []struct { + files []string + errorString string + }{ + {[]string{ + "./merge_testfiles/file1.yml", + "./merge_testfiles/badversion.yml", + }, "failed to merge ./merge_testfiles/badversion.yml: files are incompatible; " + + "major versions are incompatible; 3.0 and 1.0"}, + {[]string{ + "./merge_testfiles/file1.yml", + "./merge_testfiles/transform_false.yml", + }, "failed to merge ./merge_testfiles/transform_false.yml: files are incompatible; " + + "files with '_transform: true' (default) and '_transform: false' are not compatible"}, + } + + for _, tdata := range testSet { + _, err := Files(tdata.files) + assert.EqualError(t, err, tdata.errorString) + } +} diff --git a/merge/merge_testfiles/badversion.yml b/merge/merge_testfiles/badversion.yml new file mode 100644 index 0000000..60e2800 --- /dev/null +++ b/merge/merge_testfiles/badversion.yml @@ -0,0 +1,3 @@ +# this version differs by major, and hence is incompatible +_format_version: "1.0" +_transform: true diff --git a/merge/merge_testfiles/file1.yml b/merge/merge_testfiles/file1.yml new file mode 100644 index 0000000..c73c0eb --- /dev/null +++ b/merge/merge_testfiles/file1.yml @@ -0,0 +1,30 @@ +# Metadata fields start with an underscore (_) +# Fields that do not start with an underscore represent Kong entities and attributes + +_comment: this is file1 + +# _format_version is mandatory, +# it specifies the minimum version of Kong that supports the format +_format_version: "3.0" + +# _transform is optional, defaulting to true. +# It specifies whether schema transformations should be applied when importing this file +# as a rule of thumb, leave this setting to true if you are importing credentials +# with plain passwords, which need to be encrypted/hashed before storing on the database. +# On the other hand, if you are reimporting a database with passwords already encrypted/hashed, +# set it to false. +_transform: true + +services: +- name: file1-service-1 + url: http://example.com + routes: + - name: my_route + paths: + - /path +- name: file1-service-2 + url: https://example.org + +routes: +- name: file1-route-1 + hosts: ["hello.com"] diff --git a/merge/merge_testfiles/file2.yml b/merge/merge_testfiles/file2.yml new file mode 100644 index 0000000..6f6eebe --- /dev/null +++ b/merge/merge_testfiles/file2.yml @@ -0,0 +1,15 @@ +_comment: this is file2 + +# different minor version +_format_version: "3.1" +#_transform: true this should default to true if omitted + +services: +- name: file2-service-1 + url: http://example.com +- name: file2-service-2 + url: https://example.org + +routes: +- name: file2-route-1 + hosts: ["hello.com"] diff --git a/merge/merge_testfiles/file3.yml b/merge/merge_testfiles/file3.yml new file mode 100644 index 0000000..eea6dbc --- /dev/null +++ b/merge/merge_testfiles/file3.yml @@ -0,0 +1,17 @@ +_comment: this is file3 + +# If omitted, defaults to "0.0" in the order, but compatible with all versions. +# Officially this is a required field. +#_format_version: "3.0" + +_transform: true + +services: +- name: file3-service-1 + url: http://example.com +- name: file3-service-2 + url: https://example.org + +routes: +- name: file3-route-1 + hosts: ["hello.com"] diff --git a/merge/merge_testfiles/test1_expected.json b/merge/merge_testfiles/test1_expected.json new file mode 100644 index 0000000..05b1c29 --- /dev/null +++ b/merge/merge_testfiles/test1_expected.json @@ -0,0 +1,31 @@ +{ + "_comment": "this is file3", + "_format_version":"3.1", + "_transform":true, + "routes":[ + { + "hosts": ["hello.com"], + "name": "file1-route-1" + }, { + "hosts": ["hello.com"], + "name": "file2-route-1" + }, { + "hosts": ["hello.com"], + "name": "file3-route-1" + } + ], + "services":[ + { "name":"file1-service-1", + "url":"http://example.com", + "routes": [{ + "name": "my_route", + "paths": ["/path"] + }] + }, + {"name":"file1-service-2", "url":"https://example.org"}, + {"name":"file2-service-1", "url":"http://example.com"}, + {"name":"file2-service-2", "url":"https://example.org"}, + {"name":"file3-service-1", "url":"http://example.com"}, + {"name":"file3-service-2", "url":"https://example.org"} + ] +} diff --git a/merge/merge_testfiles/test2_expected.json b/merge/merge_testfiles/test2_expected.json new file mode 100644 index 0000000..cad5d0d --- /dev/null +++ b/merge/merge_testfiles/test2_expected.json @@ -0,0 +1,31 @@ +{ + "_comment": "this is file1", + "_format_version":"3.1", + "_transform":true, + "routes":[ + { + "hosts": ["hello.com"], + "name": "file3-route-1" + }, { + "hosts": ["hello.com"], + "name": "file2-route-1" + }, { + "hosts": ["hello.com"], + "name": "file1-route-1" + } + ], + "services":[ + {"name":"file3-service-1", "url":"http://example.com"}, + {"name":"file3-service-2", "url":"https://example.org"}, + {"name":"file2-service-1", "url":"http://example.com"}, + {"name":"file2-service-2", "url":"https://example.org"}, + { "name":"file1-service-1", + "url":"http://example.com", + "routes": [{ + "name": "my_route", + "paths": ["/path"] + }] + }, + {"name":"file1-service-2", "url":"https://example.org"} + ] +} diff --git a/merge/merge_testfiles/transform_false.yml b/merge/merge_testfiles/transform_false.yml new file mode 100644 index 0000000..d61e744 --- /dev/null +++ b/merge/merge_testfiles/transform_false.yml @@ -0,0 +1,3 @@ +# transform 'false' cannot be merged with 'true'/null +_format_version: "1.0" +_transform: false