Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid configuration because of empty fields #11

Closed
Toilal opened this issue Dec 5, 2015 · 2 comments
Closed

Invalid configuration because of empty fields #11

Toilal opened this issue Dec 5, 2015 · 2 comments
Labels

Comments

@Toilal
Copy link
Contributor

Toilal commented Dec 5, 2015

When a field is empty, it should not be added to REST request.

As a consequence, ip-restriction plugin doesn't work properly when configured with kong-dashboard. If I configure with the whitelist only, here's the resulting configuration:

{
  "api_id": "7a6f3a89-cacc-4dfa-cdff-220757de3ff9",
  "id": "6b8e5ed1-865f-49b1-c9e9-ac7fa5cd122a",
  "created_at": 1449353915000,
  "enabled": true,
  "name": "ip-restriction",
  "config": {
    "blacklist": {},
    "whitelist": [
      "172.17.0.1"
    ],
    "_whitelist_cache": {},
    "_blacklist_cache": {}
  }
}

As you can see, blacklist is created with an empty object, and it makes the plugin consider there's a blacklist defined.

Also, I think _whitelist_cache and _blacklist_cache are added by kong-dashboard too, but is not required.

@PGBI
Copy link
Owner

PGBI commented Dec 7, 2015

Thanks for reporting, i'll have a look at this. But I remember having changed my mind several times regarding empty fields. When updating a plugin for instance, there are cases where you cannot reset an attribute using a PUT request unless you submit an empty string.

I'm wondering if it would not be better to create an issue on kong repository for this plugin. With such a configuration:

"config": {
    "blacklist": {},
    "whitelist": [
      "172.17.0.1"
    ]
}

the plugin should not consider there is any blacklist.

@PGBI PGBI added the bug label Dec 7, 2015
PGBI added a commit that referenced this issue Dec 10, 2015
@PGBI
Copy link
Owner

PGBI commented Dec 10, 2015

@Toilal should be fixed by 68b1a84

@PGBI PGBI closed this as completed Dec 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants