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.

Sign up for GitHub

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

Jump to bottom

feat(runloop): plugin:configure(configs) handler #11703

Merged
merged 1 commit into from
Oct 18, 2023
Merged

feat(runloop): plugin:configure(configs) handler #11703

merged 1 commit into from
Oct 18, 2023

Conversation

bungle
Copy link
Member

@bungle bungle commented Oct 6, 2023 •
edited
Loading

Summary

Adds a new handler that plugins can implement:

 -- configs will be `nil` if there is no active configs for the plugin
function Plugin:configure(configs)
end

See the change in acme and prometheus plugins.

KAG-2672
KAG-2678
KAG-2679

jschmid1
jschmid1 reviewed Oct 9, 2023
View reviewed changes
Copy link
Contributor

@jschmid1 jschmid1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great change!

kong/runloop/plugins_iterator.lua Outdated Show resolved Hide resolved
kong/runloop/plugins_iterator.lua Outdated Show resolved Hide resolved
kong/runloop/plugins_iterator.lua Show resolved Hide resolved
kong/runloop/plugins_iterator.lua Show resolved Hide resolved
kong/runloop/plugins_iterator.lua Outdated Show resolved Hide resolved
@kikito kikito added this to the 3.5.0 milestone Oct 10, 2023
@kikito kikito requested a review from gszr October 10, 2023 17:02
@bungle bungle force-pushed the feat/plugins-configure branch 12 times, most recently from d82ee05 to 2235663 Compare October 12, 2023 10:58
@bungle
Copy link
Member Author

bungle commented Oct 12, 2023

@jschmid1 I also tried to minimize the delta between CE and EE on this, see:
https://github.com/Kong/kong-ee/pull/6795

@bungle bungle force-pushed the feat/plugins-configure branch 7 times, most recently from 33870d9 to 088c845 Compare October 17, 2023 14:55
@bungle bungle force-pushed the feat/plugins-configure branch 2 times, most recently from 26d3be8 to b45adf6 Compare October 17, 2023 16:40
@bungle bungle force-pushed the feat/plugins-configure branch 3 times, most recently from a672f87 to 292c7b4 Compare October 18, 2023 07:47
@bungle bungle marked this pull request as ready for review October 18, 2023 07:47
jschmid1
jschmid1 reviewed Oct 18, 2023
View reviewed changes
Copy link
Contributor

@jschmid1 jschmid1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We absolutely need tests for this. What about utilizing one of our mock/test plugins and implement a configure phase that is easily verifiable?

kong/plugins/acme/handler.lua Show resolved Hide resolved
kong/plugins/prometheus/exporter.lua Show resolved Hide resolved
kong/runloop/handler.lua Show resolved Hide resolved
kong/runloop/plugins_iterator.lua Show resolved Hide resolved
kong/runloop/plugins_iterator.lua Show resolved Hide resolved
kong/runloop/plugins_iterator.lua Show resolved Hide resolved
@bungle
feat(runloop): plugin:configure(configs) handler
5ce96a4 
### Summary

Adds a new handler that plugins can implement:

```
 -- configs will be `nil` if there is no active configs for the plugin
function Plugin:configure(configs)
end
```

See the change in `acme` plugin.

Signed-off-by: Aapo Talvensaari <[email protected]>
@jschmid1 jschmid1 self-requested a review October 18, 2023 13:09
jschmid1
jschmid1 approved these changes Oct 18, 2023
View reviewed changes
Copy link
Contributor

@jschmid1 jschmid1 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all my points have been addressed. approved granted the tests pass. @ADD-SP wanted to take a look as well?

@bungle bungle merged commit be3a5ac into master Oct 18, 2023
23 checks passed
@bungle bungle deleted the feat/plugins-configure branch October 18, 2023 14:11
@bungle bungle mentioned this pull request Nov 6, 2023
Merged
2 tasks
@StarlightIbuki StarlightIbuki mentioned this pull request Feb 1, 2024
Open
@tobiasehlert tobiasehlert mentioned this pull request Feb 2, 2024
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jschmid1 jschmid1 jschmid1 approved these changes

@hanshuebner hanshuebner Awaiting requested review from hanshuebner

@gszr gszr Awaiting requested review from gszr

Assignees

@bungle bungle

Labels
core/proxy plugins/acme plugins/prometheus size/XL
Projects
None yet
Milestone
3.5.0
Development

Successfully merging this pull request may close these issues.
3 participants
@bungle @jschmid1 @kikito