Skip to content

Commit

Permalink
fix(*): avoid setting nil for missing fields
Browse files Browse the repository at this point in the history 
upon filling defaults for the configuration, the existing logic was
setting `nil` on missing fields, but in Kong Gateway, explicit `nil`
are not treated as a synonym of "missing field".
This change removes any missing fields from the configuration instead of
explicitly setting such fields as `nil`.
  • Loading branch information
@samugi
samugi committed Aug 21, 2024
1 parent 77f320b commit 0492afb
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 63 deletions.
6 changes: 0 additions & 6 deletions kong/plugin_service_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,6 @@ func TestFillPluginDefaults(T *testing.T) {
Name: String("basic-auth"),
RunOn: String("test"),
Config: Configuration{
"anonymous": nil,
"hide_credentials": false,
// NOTE: realm has been introduced in 3.6 basic auth schema
// https://docs.konghq.com/hub/kong-inc/basic-auth/changelog/#kong-gateway-36x
Expand All @@ -594,7 +593,6 @@ func TestFillPluginDefaults(T *testing.T) {
Name: String("basic-auth"),
RunOn: String("test"),
Config: Configuration{
"anonymous": nil,
"hide_credentials": false,
},
Protocols: []*string{String("grpc"), String("grpcs"), String("http"), String("https")},
Expand All @@ -619,7 +617,6 @@ func TestFillPluginDefaults(T *testing.T) {
ID: String("3bb9a73c-a467-11ec-b909-0242ac120002"),
},
Config: Configuration{
"anonymous": nil,
"hide_credentials": true,
// NOTE: realm has been introduced in 3.6 basic auth schema
// https://docs.konghq.com/hub/kong-inc/basic-auth/changelog/#kong-gateway-36x
Expand Down Expand Up @@ -647,7 +644,6 @@ func TestFillPluginDefaults(T *testing.T) {
ID: String("3bb9a73c-a467-11ec-b909-0242ac120002"),
},
Config: Configuration{
"anonymous": nil,
"hide_credentials": true,
},
Protocols: []*string{String("grpc"), String("grpcs"), String("http"), String("https")},
Expand Down Expand Up @@ -678,7 +674,6 @@ func TestFillPluginDefaults(T *testing.T) {
expected: &Plugin{
Name: String("request-transformer"),
Config: Configuration{
"http_method": nil,
"add": map[string]interface{}{
"body": []interface{}{},
"headers": "x-new-header:value",
Expand All @@ -703,7 +698,6 @@ func TestFillPluginDefaults(T *testing.T) {
"body": []interface{}{},
"headers": []interface{}{},
"querystring": []interface{}{},
"uri": nil,
},
},
Protocols: []*string{String("grpc"), String("grpcs")},
Expand Down
4 changes: 2 additions & 2 deletions kong/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -393,8 +393,8 @@ func fillConfigRecord(schema gjson.Result, config Configuration) Configuration {
if value.Exists() {
res[fname] = value.Value()
} else {
// if no default exists, set an explicit nil
res[fname] = nil
// if no default exists, remove the field
delete(res, fname)
}
return true
})
Expand Down
73 changes: 18 additions & 55 deletions kong/utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1811,7 +1811,6 @@ func Test_fillConfigRecord(t *testing.T) {
"enabled": true,
"mappings": []any{
Configuration{
"name": nil,
"nationality": "Ethiopian",
},
},
Expand All @@ -1833,7 +1832,6 @@ func Test_fillConfigRecord(t *testing.T) {
"enabled": true,
"mappings": []any{
Configuration{
"name": nil,
"nationality": "Ethiopian",
},
},
Expand Down Expand Up @@ -1964,17 +1962,14 @@ func Test_fillConfigRecord_shorthand_fields(t *testing.T) {
"enabled": true,
"mappings": []any{
Configuration{
"name": nil,
"nationality": "Ethiopian",
},
},
"empty_record": map[string]any{},
"redis": map[string]interface{}{
"host": nil,
"port": float64(6379),
},
"redis_port": float64(6379),
"redis_host": nil,
},
},
{
Expand All @@ -1986,7 +1981,6 @@ func Test_fillConfigRecord_shorthand_fields(t *testing.T) {
},
expected: Configuration{
"enabled": true,
"mappings": nil,
"empty_record": map[string]any{},
"redis": map[string]interface{}{
"host": "localhost",
Expand Down Expand Up @@ -2085,10 +2079,8 @@ func Test_FillPluginsDefaults(t *testing.T) {
"prefix": "kong",
"metrics": []interface{}{
Configuration{
"name": "response_size",
"stat_type": "histogram",
"consumer_identifier": nil,
"sample_rate": nil,
"name": "response_size",
"stat_type": "histogram",
},
},
},
Expand Down Expand Up @@ -2175,10 +2167,9 @@ func Test_FillPluginsDefaults_RequestTransformer(t *testing.T) {
"headers": []any{},
"querystring": []any{},
},
"http_method": nil,
"enabled": true,
"id": "0beef60e-e7e3-40f8-ac47-f6a10b931cee",
"name": "request-transformer",
"enabled": true,
"id": "0beef60e-e7e3-40f8-ac47-f6a10b931cee",
"name": "request-transformer",
"protocols": []any{
"grpc",
"grpcs",
Expand Down Expand Up @@ -2206,13 +2197,12 @@ func Test_FillPluginsDefaults_RequestTransformer(t *testing.T) {
"headers": []interface{}{},
"querystring": []interface{}{},
},
"remove": map[string]any{"body": []any{}, "headers": []any{}, "querystring": []any{}},
"rename": map[string]any{"body": []any{}, "headers": []any{}, "querystring": []any{}},
"replace": map[string]any{"body": []any{}, "headers": []any{}, "querystring": []any{}, "uri": nil},
"http_method": nil,
"enabled": true,
"id": "0beef60e-e7e3-40f8-ac47-f6a10b931cee",
"name": "request-transformer",
"remove": map[string]any{"body": []any{}, "headers": []any{}, "querystring": []any{}},
"rename": map[string]any{"body": []any{}, "headers": []any{}, "querystring": []any{}},
"replace": map[string]any{"body": []any{}, "headers": []any{}, "querystring": []any{}},
"enabled": true,
"id": "0beef60e-e7e3-40f8-ac47-f6a10b931cee",
"name": "request-transformer",
"protocols": []any{
"grpc",
"grpcs",
Expand Down Expand Up @@ -2381,54 +2371,29 @@ func Test_FillPluginsDefaults_Acme(t *testing.T) {
},
expected: &Plugin{
Config: Configuration{
"account_email": nil,
"account_key": nil,
"allow_any_domain": bool(false),
"api_uri": string("https://acme-v02.api.letsencrypt.org/directory"),
"cert_type": string("rsa"),
"domains": nil,
"eab_hmac_key": nil,
"eab_kid": nil,
"enable_ipv4_common_name": bool(true),
"fail_backoff_minutes": float64(5),
"preferred_chain": nil,
"renew_threshold_days": float64(14),
"rsa_key_size": float64(4096),
"storage": string("shm"),
"storage_config": map[string]any{
"consul": map[string]any{
"host": nil,
"https": bool(false),
"kv_path": nil,
"port": nil,
"timeout": nil,
"token": nil,
"https": bool(false),
},
"kong": map[string]any{},
"redis": map[string]any{
"auth": nil,
"database": nil,
"host": nil,
"namespace": string(""),
"port": nil,
"ssl": bool(false),
"ssl_server_name": nil,
"ssl_verify": bool(false),
"namespace": string(""),
"ssl": bool(false),
"ssl_verify": bool(false),
},
"shm": map[string]any{"shm_name": string("kong")},
"vault": map[string]any{
"auth_method": string("token"),
"auth_path": nil,
"auth_role": nil,
"host": nil,
"https": bool(false),
"jwt_path": nil,
"kv_path": nil,
"port": nil,
"timeout": nil,
"tls_server_name": nil,
"tls_verify": bool(true),
"token": nil,
"auth_method": string("token"),
"https": bool(false),
"tls_verify": bool(true),
},
},
"tos_accepted": bool(false),
Expand Down Expand Up @@ -2476,7 +2441,6 @@ func Test_FillPluginsDefaults_DefaultRecord(t *testing.T) {
"endpoint": "http://test.test:4317",
"propagation": map[string]interface{}{
"default_format": string("w3c"), // from record defaults
"extract": nil, // from field defaults
},
"queue": map[string]interface{}{
"max_batch_size": float64(200), // from record defaults
Expand All @@ -2503,7 +2467,6 @@ func Test_FillPluginsDefaults_DefaultRecord(t *testing.T) {
"endpoint": "http://test.test:4317",
"propagation": map[string]interface{}{
"default_format": string("b3"),
"extract": nil, // from field defaults
},
"queue": map[string]interface{}{
"max_batch_size": float64(123),
Expand Down

0 comments on commit 0492afb

Please sign in to comment.