Feat/global plugins #112
Merged
Feat/global plugins #112
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
This is part of a patchset for enabling global plugins to be created in Kong via the ingress controller.
This is part of a patchset introducing support for global plugins in Kong via the ingress controller. This is a backwards compatible change. This commit will have long-term effects in how plugins are configured via Ingress and custom resources as explained below: Currently, to apply a plugin to one has to add an annotation of the form: ``` rate-limiting.plugin.konghq.com: plugin-conf ``` where `rate-limiting` is the name of the plugin that needs to be created while `plugin-conf` is the plugin conf that should be used for that plugin. This is not as user-friendly as it should be, especially considering that different kinds of plugins in Kong have different configuration objects. This allows us to keep configuration and plugin names together and in future the annotations could be much more compact and easier to pass around: ``` plugin.konghq.com: plugin-conf1,plugin-conf2 ```
Codecov Report
@@ Coverage Diff @@
## master #112 +/- ##
==========================================
- Coverage 32.09% 29.37% -2.73%
==========================================
Files 33 33
Lines 2891 3183 +292
==========================================
+ Hits 928 935 +7
- Misses 1850 2135 +285
Partials 113 113
Continue to review full report at Codecov.
|
fffonion
reviewed
Sep 6, 2018
fffonion
reviewed
Sep 6, 2018
| } | ||
| if _, ok := targetPluginMap[name]; ok { | ||
| glog.Error("Multiple KongPlugin definitions found with 'global' annotation for :", name, | ||
| ", the plugin will not be applied") |
There was a problem hiding this comment.
Since the behavior here is to not load all plugins with the same name (as in L147-150, if i understand if correctly), should we also put that into the error log (and further more in the doc)?
Something like Multiple KongPlugin definitions with same name found with 'global' annotation, all plugin defined with name '", name, "' will not be loaded
There was a problem hiding this comment.
I think the current error message makes sense as it conveys the essence of the error.
I'll be putting in docs later in a separate PR.
fffonion
reviewed
Sep 6, 2018
There are cases where a plugin(s) should be executed for all proxied requests. In order to do this with the existing model, it becomes cumbersome to configure it for 100s of different services and would make sense to configure globally for all services. This feature is already supported by Kong. With this change, all KongPlugin resources that have a label of `"global" : "true"` will be setup as global plugins in Kong by the Ingress controller.
hbagdi
force-pushed
the
feat/global-plugins
branch
from
36053e9 to
5c5ccb6
Compare
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.
There are cases where a plugin(s) should be executed for all proxied
requests. In order to do this with the existing model, it becomes
cumbersome to configure it for 100s of different services and would make
sense to configure globally for all services. This feature is already
supported by Kong.
With this change, all KongPlugin resources that have a label of
"global" : "true"will be setup as global plugins in Kongby the Ingress controller.