feat: reuse KongPlugin resources across ingress resources #121
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #121 +/- ##
=========================================
+ Coverage 31.39% 31.59% +0.2%
=========================================
Files 33 33
Lines 2956 2937 -19
=========================================
Hits 928 928
+ Misses 1915 1896 -19
Partials 113 113
Continue to review full report at Codecov.
|
Problem: KongPlugin custom resource defines a Plugin in Kong. Currently, a KongPlugin resource exists for every Plugin entity in Kong. The sync process ensures that KongPlugin and the corresponding Plugin entitiy in Kong are in sync and diff them as they share the same ID (UID/UUID). This is not a scalable or maintainable configuration. If there are a large (100s) number of ingress rules or services, there will be similar number of KongPlugin objects for each plugin that is enabled on those routes/services. See #6 See #83 Solution: Decouple k8s KongPlugin and Plugin entities in Kong, there-by removing the unnecessary duplication. This solution implies that there is not a KongPlugin resource in k8s for every Plugin entity in Kong. This is fine as long as there can exists only a single plugin of any type (type is key-auth, rate-limiting etc), which is the case in Kong. The diff and sync logic has been updated to compare the correct plugin entity in Kong. This makes a KongPlugin resource much more usable. If one wants to change a configuration of plugin across multiple ingress, changing a single KongPlugin resource will change it in all the ingress rules (or routes/services in Kong's terminology).
hbagdi
force-pushed
the
feat/reuse-kong-plugin-cr
branch
from
b1f4cb5
to
8490c94
Compare
hbagdi
commented
Sep 11, 2018
| p := kongadminv1.Plugin{ | ||
| Name: pluginInKong.Name, | ||
| Config: kongadminv1.Configuration(pluginInk8s.Config), | ||
| Enabled: pluginInk8s.Disabled, |
There was a problem hiding this comment.
Need to check if this should be !.
fffonion
reviewed
Sep 11, 2018
fffonion
reviewed
Sep 11, 2018
| } else { | ||
| // Plugins present in Kong and k8s need should have same configuration | ||
| if !pluginDeepEqual(pluginInk8s.Config, &pluginInKong) || | ||
| (!pluginInk8s.Disabled != pluginInKong.Enabled) || |
There was a problem hiding this comment.
should this be disabled == enabled that indicates they are not same? oh i missed the ! at front, does it make sense to just be pluginInk8s.Disabled == pluginInKong.Enabled?
There was a problem hiding this comment.
This Enabled and Disabled is pretty confusing and extremely easy to trip over.
I plan to refactor it at some point but for now, I'll make this change.
|
Should we abstract the logic of comparing plugins in routes and services into a seperate function? Seems like most of the code are same. |
fffonion
approved these changes
Sep 11, 2018
This was referenced Sep 11, 2018
hbagdi
added a commit
that referenced
this pull request
Oct 23, 2018
Problem: KongPlugin custom resource defines a Plugin in Kong. Currently, a KongPlugin resource exists for every Plugin entity in Kong. The sync process ensures that KongPlugin and the corresponding Plugin entitiy in Kong are in sync and diff them as they share the same ID (UID/UUID). This is not a scalable or maintainable configuration. If there are a large (100s) number of ingress rules or services, there will be similar number of KongPlugin objects for each plugin that is enabled on those routes/services. See #6 See #83 Solution: Decouple k8s KongPlugin and Plugin entities in Kong, there-by removing the unnecessary duplication. This solution implies that there is not a KongPlugin resource in k8s for every Plugin entity in Kong. This is fine as long as there can exists only a single plugin of any type (type is key-auth, rate-limiting etc), which is the case in Kong. The diff and sync logic has been updated to compare the correct plugin entity in Kong. This makes a KongPlugin resource much more usable. If one wants to change a configuration of plugin across multiple ingress, changing a single KongPlugin resource will change it in all the ingress rules (or routes/services in Kong's terminology). From #121
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem:
KongPlugin custom resource defines a Plugin in Kong.
Currently, a KongPlugin resource exists for every Plugin entity in Kong.
The sync process ensures that KongPlugin and the corresponding Plugin
entitiy in Kong are in sync and diff them as they share the same ID
(UID/UUID). This is not a scalable or maintainable configuration. If
there are a large (100s) number of ingress rules or services, there will
be similar number of KongPlugin objects for each plugin that is enabled
on those routes/services.
See #6
See #83
Solution:
Decouple k8s KongPlugin and Plugin entities in Kong, there-by removing
the unnecessary duplication.
This solution implies that there is not a KongPlugin resource in k8s for every
Plugin entity in Kong. This is fine as long as there can exists only a
single plugin of any type (type is key-auth, rate-limiting etc), which
is the case in Kong.
The diff and sync logic has been updated to compare
the correct plugin entity in Kong.
This makes a KongPlugin resource much more usable. If one wants to
change a configuration of plugin across multiple ingress, changing a
single KongPlugin resource will change it in all the ingress rules (or
routes/services in Kong's terminology).