fix(o2k): use regex_priority in defaults if given (#143)

This default should be left untouched unless you know what you are doing
(same as strip_path). But the value will be honored now if given.
The default is assumed to be the plain-path value. A parameterized
path should have a lower precedence, hence if the value is given
a parameterized path will get the priority-1.

docs/learnservice_oas.yaml

@@ -111,6 +111,8 @@ x-kong-route-defaults:
   preserve_host: true
   # NOTE: these defaults can also be added to "path" and "operation" objects as well
   # to only apply to that subset of the spec.
+  # Fields `regex_priority` and `strip_path` should not be set. If provided they will
+  # be used, but verify the results carefully as setting them can cause unexpected results!
 
 
 paths:

jsonbasics/jsonbasics.go

@@ -161,6 +161,8 @@ func GetStringField(object map[string]interface{}, fieldName string) (string, er
 	return "", fmt.Errorf("expected key '%s' to be a string, got %t", fieldName, value)
 }
 
+// GetStringIndex returns a string-value from an array index. Returns an error if the entry
+// is not a string.
 func GetStringIndex(arr []interface{}, index int) (string, error) {
 	value := arr[index]
 	switch result := value.(type) {
@@ -181,6 +183,8 @@ func GetBoolField(object map[string]interface{}, fieldName string) (bool, error)
 	return false, fmt.Errorf("expected key '%s' to be a boolean", fieldName)
 }
 
+// GetBoolIndex returns a boolean-value from an array index. Returns an error if the entry
+// is not a boolean.
 func GetBoolIndex(arr []interface{}, index int) (bool, error) {
 	value := arr[index]
 	switch result := value.(type) {
@@ -190,6 +194,96 @@ func GetBoolIndex(arr []interface{}, index int) (bool, error) {
 	return false, fmt.Errorf("expected index '%d' to be a boolean", index)
 }
 
+// GetUInt64Field returns a uint64 from an object field. Returns an error if the field
+// is not a unsigned integer, or is not found.
+func GetUInt64Field(object map[string]interface{}, fieldName string) (uint64, error) {
+	value, err := GetFloat64Field(object, fieldName)
+	if err == nil {
+		if value == float64(int(value)) {
+			return uint64(value), nil
+		}
+	}
+	return 0, fmt.Errorf("expected key '%s' to be an unsigned integer", fieldName)
+}
+
+// GetUInt64Index returns a uint64-value from an array index. Returns an error if the entry
+// is not an unsigned integer.
+func GetUInt64Index(arr []interface{}, index int) (uint64, error) {
+	value, err := GetFloat64Index(arr, index)
+	if err == nil {
+		if value == float64(int(value)) {
+			return uint64(value), nil
+		}
+	}
+	return 0, fmt.Errorf("expected index '%d' to be an unsigned integer", index)
+}
+
+// GetFloat64Field returns a float64 from an object field. Returns an error if the field
+// is not a float, or is not found.
+func GetFloat64Field(object map[string]interface{}, fieldName string) (float64, error) {
+	value := object[fieldName]
+	switch result := value.(type) {
+	case int:
+		return float64(result), nil
+	case int8:
+		return float64(result), nil
+	case int16:
+		return float64(result), nil
+	case int32:
+		return float64(result), nil
+	case int64:
+		return float64(result), nil
+	case uint:
+		return float64(result), nil
+	case uint8:
+		return float64(result), nil
+	case uint16:
+		return float64(result), nil
+	case uint32:
+		return float64(result), nil
+	case uint64:
+		return float64(result), nil
+	case float32:
+		return float64(result), nil
+	case float64:
+		return result, nil
+	}
+	return 0, fmt.Errorf("expected key '%s' to be a float", fieldName)
+}
+
+// GetFloat64Index returns a float64-value from an array index. Returns an error if the entry
+// is not a float.
+func GetFloat64Index(arr []interface{}, index int) (float64, error) {
+	value := arr[index]
+	switch result := value.(type) {
+	case int:
+		return float64(result), nil
+	case int8:
+		return float64(result), nil
+	case int16:
+		return float64(result), nil
+	case int32:
+		return float64(result), nil
+	case int64:
+		return float64(result), nil
+	case uint:
+		return float64(result), nil
+	case uint8:
+		return float64(result), nil
+	case uint16:
+		return float64(result), nil
+	case uint32:
+		return float64(result), nil
+	case uint64:
+		return float64(result), nil
+	case float32:
+		return float64(result), nil
+	case float64:
+		return result, nil
+	}
+	return 0, fmt.Errorf("expected index '%d' to be a float", index)
+}
+
 // DeepCopyObject implements a poor man's deepcopy by jsonify/de-jsonify
 func DeepCopyObject(data map[string]interface{}) map[string]interface{} {
 	var dataCopy map[string]interface{}

logbasics/logbasics.go

@@ -32,7 +32,8 @@ func Debug(msg string, keysAndValues ...interface{}) {
 	globalLogger.V(2).Info(msg, keysAndValues...)
 }
 
-// Error logs an error message.
+// Error logs an error message. Preferably errors should bubble up to the caller,
+// so only if that is not possible, this method should be used.
 func Error(err error, msg string, keysAndValues ...interface{}) {
 	globalLogger.Error(err, msg, keysAndValues...)
 }

openapi2kong/oas3_testfiles/08-route-defaults-overrides.expected.json

@@ -20,7 +20,24 @@
             "~/path1$"
           ],
           "plugins": [],
-          "regex_priority": 200,
+          "regex_priority": 100,
+          "strip_path": false,
+          "tags": [
+            "OAS3_import",
+            "OAS3file_08-route-defaults-overrides.yaml"
+          ]
+        },
+        {
+          "id": "b6ab5ad9-ed23-5957-9d57-d071678162d1",
+          "methods": [
+            "GET"
+          ],
+          "name": "simple-api-overview_uses-doc-defaults-with-path-param",
+          "paths": [
+            "~/path1/(?<param>[^#?/]+)$"
+          ],
+          "plugins": [],
+          "regex_priority": 99,
           "strip_path": false,
           "tags": [
             "OAS3_import",
@@ -54,7 +71,7 @@
             "~/path2$"
           ],
           "plugins": [],
-          "regex_priority": 200,
+          "regex_priority": 300,
           "strip_path": true,
           "tags": [
             "OAS3_import",

openapi2kong/oas3_testfiles/08-route-defaults-overrides.yaml

@@ -20,6 +20,16 @@ paths:
         '200':
           description: |-
             200 response
+  /path1/{param}:
+    get:
+      # should get the document level defaults, but with a path-parameter the
+      # regex_priority is set to 1 less than the defaults given
+      operationId: uses-doc-defaults-with-path-param
+      summary: List API versions
+      responses:
+        '200':
+          description: |-
+            200 response
   /path2:
     # specify new defaults to override document level
     x-kong-route-defaults:

openapi2kong/oas3_testfiles/11-references.expected.json

@@ -33,7 +33,7 @@
             "~/path1$"
           ],
           "plugins": [],
-          "regex_priority": 200,
+          "regex_priority": 999,
           "strip_path": false,
           "tags": [
             "OAS3_import",

openapi2kong/openapi2kong.go

@@ -20,6 +20,10 @@ import (
 const (
 	formatVersionKey   = "_format_version"
 	formatVersionValue = "3.0"
+
+	// default regex priorities to assign to routes
+	regexPriorityWithPathParams = 100
+	regexPriorityPlain          = 200 // non-regexed (no params) paths have higher precedence in OAS
 )
 
 // O2KOptions defines the options for an O2K conversion operation
@@ -1063,9 +1067,9 @@ func Convert(content []byte, opts O2kOptions) (map[string]interface{}, error) {
 
 			// convert path parameters to regex captures
 			re, _ := regexp.Compile("{([^}]+)}")
-			regexPriority := 200 // non-regexed (no params) paths have higher precedence in OAS
+			regexPriority := regexPriorityPlain
 			if matches := re.FindAllStringSubmatch(convertedPath, -1); matches != nil {
-				regexPriority = 100
+				regexPriority = regexPriorityWithPathParams
 				for _, match := range matches {
 					varName := match[1]
 					// match single segment; '/', '?', and '#' can mark the end of a segment
@@ -1083,7 +1087,20 @@ func Convert(content []byte, opts O2kOptions) (map[string]interface{}, error) {
 			route["name"] = operationBaseName
 			route["methods"] = []string{method}
 			route["tags"] = kongTags
-			route["regex_priority"] = regexPriority
+			if _, found := route["regex_priority"]; !found {
+				route["regex_priority"] = regexPriority
+			} else {
+				// a regex_priority was provided in the defaults
+				currentRegexPrio, err := jsonbasics.GetUInt64Field(route, "regex_priority")
+				if err != nil {
+					return nil, fmt.Errorf("failed to parse 'regex_priority' from route defaults: %w", err)
+				}
+				// the default in x-kong-route-defaults represents the plain path, path-parameter path needs to be lower
+				if regexPriority == regexPriorityWithPathParams {
+					// this is a path with parameters, so we need to lower the priority
+					route["regex_priority"] = currentRegexPrio - 1
+				}
+			}
 			if _, found := route["strip_path"]; !found {
 				route["strip_path"] = false // Default to false since we do not want to strip full-regex paths by default
 			}