Skip to content

Commit

Permalink
fix(utils): support record-level default schema values
Browse files Browse the repository at this point in the history
Before this change, during the process of filling default configuration
values, if a record existed in the schema with a structure similar to the
following:
```
{
	"default": {
			"some_field": "record_level_default"
	},
	"fields": [
		{
			"some_field": {
				"type": "string",
				"default": "field_level_default"
			}
		}
	]
}
```
the value `record_level_default` would be ignored and the field set to
the `field_level_default` value. Similarly, in case of no field-level
default, the field would be set to `null`.

This commit modifies the utils/fillConfigRecord logic to assign fields
with record-level default values when available.

Example of default configuration values that were previously ignored:
https://github.com/Kong/kong/blob/8fe14097a17cf904676672fc8cdaabe5e02e4a2d/kong/plugins/opentelemetry/schema.lua#L49
https://github.com/Kong/kong/blob/8fe14097a17cf904676672fc8cdaabe5e02e4a2d/kong/plugins/opentelemetry/schema.lua#L90
  • Loading branch information
samugi committed Mar 28, 2024
1 parent 241dbb8 commit f68d1af
Show file tree
Hide file tree
Showing 3 changed files with 218 additions and 1 deletion.
73 changes: 73 additions & 0 deletions kong/plugin_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -676,6 +676,79 @@ func TestFillPluginDefaults(T *testing.T) {
}
}

func TestFillPluginDefaultsInRecords(T *testing.T) {
RunWhenKong(T, ">=3.7.0")

client, err := NewTestClient(nil, nil)
require.NoError(T, err)
require.NotNil(T, client)

tests := []struct {
name string
plugin *Plugin
expected *Plugin
}{
{
name: "record defaults take precedence over field defaults",
plugin: &Plugin{
Name: String("opentelemetry"),
Config: Configuration{
"endpoint": "http://test.test:4317",
},
Enabled: Bool(false),
Protocols: []*string{String("http")},
},
expected: &Plugin{
Name: String("opentelemetry"),
Config: Configuration{
"endpoint": "http://test.test:4317",
"batch_flush_delay": nil,
"batch_span_count": nil,
"connect_timeout": float64(1000),
"header_type": "preserve",
"headers": nil,
"http_response_header_for_traceid": nil,
"propagation": map[string]interface{}{
"clear": nil,
"default_format": string("w3c"), // from record default
"extract": nil,
"inject": nil,
},
"queue": map[string]interface{}{
"initial_retry_delay": float64(0.01),
"max_batch_size": float64(200), // from record default
"max_bytes": nil,
"max_coalescing_delay": float64(1),
"max_entries": float64(10000),
"max_retry_delay": float64(60),
"max_retry_time": float64(60),
},
"read_timeout": float64(5000),
"resource_attributes": nil,
"sampling_rate": nil,
"send_timeout": float64(5000),
},
Protocols: []*string{String("http")},
Enabled: Bool(false),
},
},
}

for _, tc := range tests {
T.Run(tc.name, func(t *testing.T) {
p := tc.plugin
fullSchema, err := client.Plugins.GetFullSchema(defaultCtx, p.Name)
require.NoError(t, err)
require.NotNil(t, fullSchema)
require.NoError(t, FillPluginsDefaults(p, fullSchema))

if diff := cmp.Diff(p, tc.expected); diff != "" {
t.Errorf(diff)
}
})
}
}

// TestFillPluginDefaultsArbitraryMap is split from TestFillPluginDefaults due to a version compatibility issue
func TestFillPluginDefaultsArbitraryMap(T *testing.T) {
RunWhenKong(T, ">=2.3.0")
Expand Down
11 changes: 10 additions & 1 deletion kong/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ func getConfigSchema(schema gjson.Result) (gjson.Result, error) {
func fillConfigRecord(schema gjson.Result, config Configuration) Configuration {
res := config.DeepCopy()
value := schema.Get("fields")
defaultRecordValue := schema.Get("default")

value.ForEach(func(_, value gjson.Result) bool {
// get the key name
Expand Down Expand Up @@ -302,7 +303,15 @@ func fillConfigRecord(schema gjson.Result, config Configuration) Configuration {
}
}
}
value = value.Get(fname + ".default")

// Check if the record has a default value for the specified field.
// If so, use it. If not, fall back to the default value of the field itself.
if defaultRecordValue.Exists() && defaultRecordValue.Get(fname).Exists() {
value = defaultRecordValue.Get(fname)
} else {
value = value.Get(fname + ".default")
}

if value.Exists() {
res[fname] = value.Value()
} else {
Expand Down
135 changes: 135 additions & 0 deletions kong/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,91 @@ const AcmeSchema = `{
]
}`

const defaultRecordSchema = `{
"fields": [
{
"config": {
"fields": [
{
"endpoint": {
"description": "endpoint",
"referenceable": true,
"required": true,
"type": "string"
}
},
{
"queue": {
"default": {
"max_batch_size": 200
},
"fields": [
{
"max_batch_size": {
"between": [
1,
1000000
],
"default": 1,
"type": "integer"
}
},
{
"max_coalescing_delay": {
"between": [
0,
3600
],
"default": 1,
"type": "number"
}
}
],
"required": true,
"type": "record"
}
},
{
"propagation": {
"default": {
"default_format": "w3c"
},
"fields": [
{
"extract": {
"elements": {
"one_of": [
"w3c",
"b3"
],
"type": "string"
},
"type": "array"
}
},
{
"default_format": {
"one_of": [
"w3c",
"b3"
],
"required": true,
"type": "string"
}
}
],
"required": true,
"type": "record"
}
}
],
"required": true,
"type": "record"
}
}
]
}`

func TestStringArrayToString(t *testing.T) {
assert := assert.New(t)

Expand Down Expand Up @@ -2213,6 +2298,56 @@ func Test_FillPluginsDefaults_Acme(t *testing.T) {
}
}

func Test_FillPluginsDefaults_DefaultRecord(t *testing.T) {
client, err := NewTestClient(nil, nil)
require.NoError(t, err)
require.NotNil(t, client)

tests := []struct {
name string
plugin *Plugin
expected *Plugin
}{
{
name: "record defaults take precedence over fields defaults",
plugin: &Plugin{
Config: Configuration{
"endpoint": "http://test.test:4317",
},
},
expected: &Plugin{
Config: Configuration{
"endpoint": "http://test.test:4317",
"propagation": map[string]interface{}{
"default_format": string("w3c"), // from record defaults
"extract": nil,
},
"queue": map[string]interface{}{
"max_batch_size": float64(200), // from record defaults
"max_coalescing_delay": float64(1),
},
},
},
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
plugin := tc.plugin
var fullSchema map[string]interface{}
err := json.Unmarshal([]byte(defaultRecordSchema), &fullSchema)

require.NoError(t, err)
require.NotNil(t, fullSchema)
assert.NoError(t, FillPluginsDefaults(plugin, fullSchema))
opts := cmpopts.IgnoreFields(*plugin, "Enabled", "Protocols")
if diff := cmp.Diff(plugin, tc.expected, opts); diff != "" {
t.Errorf(diff)
}
})
}
}

const NonEmptyDefaultArrayFieldSchema = `{
"fields": [
{
Expand Down

0 comments on commit f68d1af

Please sign in to comment.