Skip to content

Commit

Permalink
fix: don't overwrite fields set to empty arrays when default exists
Browse files Browse the repository at this point in the history
This commit fixes a bug preventing users to set fields to emtpy
arrays when a default for those fields exist. For example, imagine
we have an `openid-connect` plugin configured via the UI having
the `login_tokens` set to an empty array (note: this field has
a default value of `["id_token"]`:

```
$ http :8001/plugins/$id | jq .config.login_tokens
[]
```

If we do a `deck dump`, the configuration will be reflected
into a local state file:

```
$ deck dump --yes

$ cat kong.yaml | grep login_tokens
    login_tokens: []
```

But, if we now run `deck diff` we see that decK will detect
a difference because of the `login_tokens` field's default
value:

```
$ deck diff
updating plugin openid-connect (global)  {
   "config": {
     "anonymous": null,
     ...
     ...
     "login_redirect_mode": "fragment",
     "login_redirect_uri": null,
     "login_tokens": [
+      "id_token"
     ],
     "logout_methods": [
       "POST",
       "DELETE"
     ],
     ...
     ...
 }

Summary:
  Created: 0
  Updated: 1
  Deleted: 0
```

This commit corrects this defect by allowing decK to set empty
arrays when default values exist for a given field.
  • Loading branch information
GGabriele committed Oct 17, 2023
1 parent dc4db7b commit 04f9441
Show file tree
Hide file tree
Showing 2 changed files with 135 additions and 3 deletions.
5 changes: 2 additions & 3 deletions kong/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,9 +236,8 @@ func fillConfigRecord(schema gjson.Result, config Configuration) Configuration {
}
case []interface{}:
if value.Get(fname).Get("elements.type").String() != "record" &&
config[fname] != nil &&
len(config[fname].([]interface{})) > 0 {
// Non empty array with elements which are not of type record
config[fname] != nil {
// Non nil array with elements which are not of type record
// this means field is already set
return true
}
Expand Down
133 changes: 133 additions & 0 deletions kong/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2212,3 +2212,136 @@ func Test_FillPluginsDefaults_Acme(t *testing.T) {
})
}
}

const NonEmptyDefaultArrayFieldSchema = `{
"fields": [
{
"protocols": {
"default": [
"grpc",
"grpcs",
"http",
"https"
],
"elements": {
"len_min": 1,
"one_of": [
"grpc",
"grpcs",
"http",
"https"
],
"required": true,
"type": "string"
},
"required": true,
"type": "set"
}
},
{
"config": {
"fields": [
{
"issuer": {
"required": true,
"type": "string"
}
},
{
"login_tokens": {
"default": [
"id_token"
],
"elements": {
"one_of": [
"id_token",
"access_token",
"refresh_token",
"tokens",
"introspection"
],
"type": "string"
},
"required": false,
"type": "array"
}
}
],
"type": "record"
}
}
]
}`

func Test_FillPluginsDefaults_NonEmptyDefaultArrayField(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: "not setting login_tokens should be overwritten by default value",
plugin: &Plugin{
Config: Configuration{
"issuer": "https://accounts.google.com",
},
},
expected: &Plugin{
Config: Configuration{
"issuer": "https://accounts.google.com",
"login_tokens": []any{"id_token"},
},
},
},
{
name: "setting empty array for login_tokens should not be overwritten by default value",
plugin: &Plugin{
Config: Configuration{
"issuer": "https://accounts.google.com",
"login_tokens": []any{},
},
},
expected: &Plugin{
Config: Configuration{
"issuer": "https://accounts.google.com",
"login_tokens": []any{},
},
},
},
{
name: "setting non-empty login_tokens should not be overwritten by default value",
plugin: &Plugin{
Config: Configuration{
"issuer": "https://accounts.google.com",
"login_tokens": []any{"access_token", "refresh_token"},
},
},
expected: &Plugin{
Config: Configuration{
"issuer": "https://accounts.google.com",
"login_tokens": []any{"access_token", "refresh_token"},
},
},
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
plugin := tc.plugin
var fullSchema map[string]interface{}
err := json.Unmarshal([]byte(NonEmptyDefaultArrayFieldSchema), &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)
}
})
}
}

0 comments on commit 04f9441

Please sign in to comment.