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

feat: implement support for custom entities in DB-less mode #630

Merged
merged 1 commit into from
May 12, 2020

Conversation

hbagdi
Copy link
Member

@hbagdi hbagdi commented Apr 30, 2020

Problem

As of today, there is no support in the controller
for custom entities inside Kong. The core custom entities related to
auth plugins are supported out of the box to cover all features that
Kong supports out of the box.

For Kong deployments backed by a database, a custom entity can be
populated via the Admin API and the controller will leave those entities
as is. These custom entities can then be referenced in plugin
configuration. While not perfect, this works currently.

For Kong deployments running without a database, there currently is no
way to setup custom entities.

Solution

This patch adds the ability to add in custom entities into DB-less
instances via a Secret. The secret should have a JSON encoded
representation of he custom entities inside a key with the name
'config'. The controller then ensures that the custom entities are
populated into Kong. The solution only works for DB-less modes.

Caveats

The solution here addresses only a limited number of cases where the
custom entity is used by plugin configurations.
There could be a number of cases where the custom entities have a direct
relationship with core entities. Such cases are currently not supported
as the controller doesn't provide any stable UUIDs for core entities.

Future-work

  • Supporting custom entities which refer core entities of Kong can be
    added in future. As of today, I'm not aware of any use-cases that fall
    into this category.

  • Adding support for controller-managed entities for DB-mode is important
    but not supported by decK (the underlying sync library).
    decK's static nature makes it hard to extend it in any way currently.

  • There is no validation performed by the controller. In future, the
    controller can use /schemas endpoint on Kong's Admin API to perform
    validation and be more resilient.

@hbagdi hbagdi requested a review from rainest May 1, 2020 23:20
@rainest
Copy link
Contributor

rainest commented May 5, 2020

The secret should have a JSON encoded representation of he custom entities inside a key with the name 'config'.

Minor: we may want to support YAML here also, but it's trivial to add later if we want.

As of today, I'm not aware of any use-cases that fall into this category.

Any custom auth plugins that define credentials should fit this, but are presumably already covered by secret-based credentials. Do you have anything offhand that we can use for practical testing? I guess the OIDC JWKS entity works, even though it's weird to configure those statically.

The secret should have a JSON encoded representation of he custom entities inside a key with the name 'config'.

Using a single key and single secret seems to be limiting this a bit too strictly. Would it make sense to use a label-based system like we use for secret credentials? I have several concerns with the current strategy:

  • Environments may be used by multiple users with different levels of access. If teamA and teamB only have access to their respective namespaces, only one namespace can contain the entities secret, and only one team would be able to create entities. It's possible to work around this by creating a shared namespace for the entities secret only, but that both dictates some cluster security policy and provides no means for teams to isolate their configuration from one another.
  • A single secret does not allow incremental changes to configuration: if you bork your JSON, you bork every entity, and secrets don't keep revisions. Allowing multiple keys also works around this, but not the other issues, though I still like iterating over all keys in the secret to allow more incremental changes without creating a large number of secrets.
  • Individual secrets are limited to 1MiB of data. This is still pretty generous--ballpark number of entities that fit is probably somewhere in the tens of thousands--but it may still be worth considering.

@hbagdi
Copy link
Member Author

hbagdi commented May 5, 2020

Minor: we may want to support YAML here also, but it's trivial to add later if we want.

I intentionally left those out. If this really comes up, we can add it.

Do you have anything offhand that we can use for practical testing?

Enterprise rate-limiting plugin for GraphQL. There are no OSS plugins using this currently. There are some plugins by community which could use this but will take longer to search on Github.

Would it make sense to use a label-based system like we use for secret credentials?

Do you mean label based flow for CACertificates? If so, this is intentionally designed to not do that. We want the owner of the controller to configure the reference for the secret containing the secret data. The secret itself could be managed by a different team but the owner of the controller should have the authority over which secret to use. This can be made more self-service if this feature actually takes off and users start using it.

A single secret does not allow incremental changes to configuration: if you bork your JSON, you bork every entity, and secrets don't keep revisions. Allowing multiple keys also works around this, but not the other issues, though I still like iterating over all keys in the secret to allow more incremental changes without creating a large number of secrets.

I agree here and I did consider using this approach early on but decided against it for the following reason:
As I started creating this feature, I quickly realized that we are adding this for the long-tail of users who have custom entities defined for their installations. And even in those configurations, this will only work if there are stand-alone entities only.
I do agree that ideally, this should allow for multiple secrets and multiple keys inside a secret as well. My take here is that would be an over-engineered solution from what we currently know how this feature should be used.

Having said the above, it is important to figure out how the above concerns can be eliminated in future in a backwards compatible way. To do this, I think we can accomplish this in future if the need arises:

  • Label based solution can be implemented in a backwards compatible way in addition to having the feature presented with this PR
  • Re: using multiple values in the secret (instead of the hard-coded config), this is easy to add in future using more backwards-compatible addition to the existing value of the CLI argument or using annotations, or generally adding support for all keys in the secret. The names in k8s are valid DNS_LABELS, meaning we have a lot of special characters to use to implement this feature.

rainest
rainest previously approved these changes May 11, 2020
@hbagdi hbagdi force-pushed the feat/custom-entities branch 2 times, most recently from 5e681c7 to 23c8c1d Compare May 11, 2020 23:52
Problem
-------

As of today, there is no support in the controller
for custom entities inside Kong. The core custom entities related to
auth plugins are supported out of the box to cover all features that
Kong supports out of the box.

For Kong deployments backed by a database, a custom entity can be
populated via the Admin API and the controller will leave those entities
as is. These custom entities can then be referenced in plugin
configuration. While not perfect, this works currently.

For Kong deployments running without a database, there currently is no
way to setup custom entities.

Solution
--------

This patch adds the ability to add in custom entities into DB-less
instances via a Secret. The secret should have a JSON encoded
representation of he custom entities inside a key with the name
'config'. The controller then ensures that the custom entities are
populated into Kong. The solution only works for DB-less modes.

Caveats
-------

The solution here addresses only a limited number of cases where the
custom entity is used by plugin configurations.
There could be a number of cases where the custom entities have a direct
relationship with core entities. Such cases are currently not supported
as the controller doesn't provide any stable UUIDs for core entities.

Future-work
-----------

- Supporting custom entities which refer core entities of Kong can be
added in future. As of today, I'm not aware of any use-cases that fall
into this category.

- Adding support for controller-managed entities for DB-mode is important
but not supported by decK (the underlying sync library).
decK's static nature makes it hard to extend it in any way currently.

- There is no validation performed by the controller. In future, the
controller can use `/schemas` endpoint on Kong's Admin API to perform
validation and be more resilient.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants