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

Consumer information unavailable unless a consumer was managed #75

Open
tophercullen opened this issue May 31, 2017 · 2 comments
Open

Comments

@tophercullen
Copy link

tophercullen commented May 31, 2017

I'm working on a PR that allows the usage of usernames or custom_ids when configuring plugins. The idea being to map them to the kong consumer_id in that environment.

The problem I'm running into is that no consumer information is requested from the kong api (and thus available to other functions) unless there is a 'consumer' section in the config. Example broken config

apis:
  - name: myapi
    ensure: "present" 
    attributes:
      upstream_url: https://service-api/ # (required)
      hosts: []
      uris: /myapi/
      methods: []
      strip_uri: false
      preserve_host: true
      retries: 5
      upstream_connect_timeout: 60000
      upstream_read_timeout: 60000
      upstream_send_timeout: 60000
      https_only: true 
      http_if_terminated: false
    plugins:
      - name: rate-limiting 
        ensure: "present" 
        username: user1
        attributes: 
          config:
            minute: 4
      - name: rate-limiting # kong plugin name
        ensure: "present" 
        custom_id: 1c6088ba-41f0-44be-9879-a784d8683788
        attributes:
          config:
            minute: 3

This results in "Error: Unable to find consumer user1". Debugging shows there is no consumer information available. Adding any consumer config (example below) results in consumer information being available, and the code works

consumers:
  - username: fakeuser
    ensure: "removed"

This is very likely not a bug, but a nuance of javascript I simply don't understand. I'd be very appreciative if you could point me in the right direction.

@CyExy
Copy link
Contributor

CyExy commented Jun 1, 2017

This is a great idea.

To keep in mind, there is an issue with consumers because with some use-cases you would want to manage consumers outside of kongfig, and if there are no consumers defined in the config file we ignore them. This is confusing because it just changes the API call to return an empty list instead of making the call to the Kong admin. https://github.com/mybuilder/kongfig/blob/master/src/cli-apply.js#L36

Currently, the world a la actual Kong state is pre-fetched into memory every time before diffing and applying a config definition. The intention was to move to a local state that would eliminate a lot of calls to Kong admin. The problem is the lack of integration tests that would give the confidence to do so. And without the local state, we need to make lots of unnecessary calls to the Kong admin.

Maybe it is time to change how the Kong state is managed, one option is to make the world lazy e.g. return promises instead of fetching state from memory. This would mean that all world calls would need to change from world.hasApi(api.name) to await world.hasApi(api.name) etc. Testing would be harder but memory footprint and the number of calls we make to Kong admin would be greatly reduced.

@ktomk
Copy link

ktomk commented Jun 25, 2018

@CyExy: Could it be the pre-fetching of consumers which you described as being status-quo in your recent comment is limited and does not fully work? Is this perhaps already reported? (sorry for de-reailing this issue, I was not able to find a better matching one even I think this has been already reported).

/e: by default the consumers listing in the API is limited to 100 in size. Setting the size query parameter to a larger value allows to retrieve many more consumer entries docs-ref.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants