Skip to content

Commit

Permalink
fix: ensure defaults are properly filled for a set (#333)
Browse files Browse the repository at this point in the history
Co-authored-by: Travis Raines <[email protected]>
  • Loading branch information
aboudreault and rainest committed Jun 6, 2023
1 parent 482d841 commit 46c44eb
Show file tree
Hide file tree
Showing 2 changed files with 164 additions and 1 deletion.
3 changes: 2 additions & 1 deletion kong/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,8 @@ func fillConfigRecord(schema gjson.Result, config Configuration) Configuration {
// If this array is non-nil and non-empty (in Config), go through all the records in this array and add defaults
// If the array has only primitives like string/number/boolean then the value is already set
// If the array is empty or nil, then no defaults need to be set for its elements
if ftype.String() == "array" {
// The same logic should be applied if field is of type set of records (in Schema)
if ftype.String() == "array" || ftype.String() == "set" {
if value.Get(fname).Get("elements.type").String() == "record" {
if config[fname] != nil {
// Check sub config is of type array and it is non-empty
Expand Down
162 changes: 162 additions & 0 deletions kong/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,47 @@ const StatsDSchema = `{
]
}`

// TestSchemaSetType (
const TestSchemaSetType = `{
"fields": [{
"config": {
"type": "record",
"fields": [{
"bootstrap_servers": {
"default": [
{
"host": "127.0.0.1",
"port": 42
}
],
"elements": {
"fields": [{
"host": {
"required": true,
"type": "string"
}
},
{
"port": {
"between": [
0,
65535
],
"required": true,
"type": "integer",
"default": 42
}
}
],
"type": "record"
},
"type": "set"
}
}]
}
}]
}`

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

Expand Down Expand Up @@ -1509,3 +1550,124 @@ func Test_FillPluginsDefaults_RequestTransformer(t *testing.T) {
})
}
}

func Test_FillPluginsDefaults_SetType(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: "does not fill defaults when provided",
plugin: &Plugin{
Config: Configuration{
"bootstrap_servers": []interface{}{
map[string]interface{}{
"host": "192.168.2.100",
"port": float64(3500),
},
map[string]any{
"host": "192.168.2.101",
"port": float64(3500),
},
},
},
},
expected: &Plugin{
Config: Configuration{
"bootstrap_servers": []interface{}{
Configuration{
"host": "192.168.2.100",
"port": float64(3500),
},
Configuration{
"host": "192.168.2.101",
"port": float64(3500),
},
},
},
},
},
{
name: "fills defaults for all missing fields",
plugin: &Plugin{
Config: Configuration{
"bootstrap_servers": []interface{}{
map[string]interface{}{
"host": "127.0.0.1",
},
},
},
},
expected: &Plugin{
Config: Configuration{
"bootstrap_servers": []interface{}{
Configuration{
"host": "127.0.0.1",
"port": float64(42),
},
},
},
},
},
{
name: "fills defaults when empty set of records in config",
plugin: &Plugin{
Config: Configuration{
"bootstrap_servers": []any{},
},
},
expected: &Plugin{
Config: Configuration{
"bootstrap_servers": []any{
map[string]any{
"host": "127.0.0.1",
"port": float64(42),
},
},
},
},
},
{
name: "fills defaults when nil set of records in config",
plugin: &Plugin{
Config: Configuration{
"bootstrap_servers": nil,
},
},
expected: &Plugin{
Config: Configuration{
"bootstrap_servers": []any{
map[string]any{
"host": "127.0.0.1",
"port": float64(42),
},
},
},
},
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
plugin := tc.plugin

var fullSchema map[string]interface{}
err := json.Unmarshal([]byte(TestSchemaSetType), &fullSchema)
require.NoError(t, err)
require.NotNil(t, fullSchema)

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

0 comments on commit 46c44eb

Please sign in to comment.