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(test): add reconfiguration completion detection plugin #12391

Merged

Conversation

hanshuebner
Copy link
Contributor

@hanshuebner hanshuebner commented Jan 22, 2024

Summary

Unlike the previous implementation, this one does not require changes
to Kong and its proxy path. It works based on the assumption that the
order of admin API changes is preserved. The admin API client marks
the end of the changes that it needs to see propagated to the data
plane(s) by changing the configuration of this plugin, setting a
particular configuration version number. On the proxy path, a header
X-Kong-Configuration-Version is sent with that version number. The
plugin's access handler verifies that the version number configured in
the plugin (on the dataplane) matches the version number requested by
the client. If the version numbers do not match, a 503 error is
generated, which causes the client to retry.

The plugin is available only to busted tests. It needs to be enabled
when starting Kong.

A new busted test helper function make_synchronized_clients is
provided that automatically synchronizes a proxy client and an admin
API client. The the test can freely mix invocations to either
endpoints. Whenever a change is made through the admin API, the proxy
path request is delayed until the change has propagated to the data
plane. spec/02-integration/13-vaults/06-refresh-secrets_spec.lua has
been updated to use the function as an illustration.

Checklist

  • The Pull Request has tests
  • A changelog file has been created under changelog/unreleased/kong or skip-changelog label added on PR if changelog is unnecessary. README.md
  • There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - PUT DOCS PR HERE

Issue reference

KAG-3265

@hanshuebner hanshuebner force-pushed the test/reconfiguration-completion-detection-plugin branch 3 times, most recently from 94841e9 to 22fbc04 Compare January 22, 2024 13:21
@github-actions github-actions bot added the chore Not part of the core functionality of kong, but still needed label Jan 22, 2024
@hanshuebner hanshuebner force-pushed the test/reconfiguration-completion-detection-plugin branch from 22fbc04 to de990ca Compare January 22, 2024 14:58
Copy link
Member

@samugi samugi left a comment

Choose a reason for hiding this comment

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

nice and simple, looks good to me

@AndyZhang0707 AndyZhang0707 requested review from dndx and chobits January 23, 2024 03:57
dndx
dndx previously requested changes Jan 24, 2024
kong-3.6.0-0.rockspec Outdated Show resolved Hide resolved
@hanshuebner hanshuebner force-pushed the test/reconfiguration-completion-detection-plugin branch 3 times, most recently from fe7a254 to e822bc7 Compare February 7, 2024 15:33
@hanshuebner hanshuebner force-pushed the test/reconfiguration-completion-detection-plugin branch 3 times, most recently from f0dc75a to bc43602 Compare February 20, 2024 13:15
@hanshuebner hanshuebner dismissed dndx’s stale review February 20, 2024 13:17

Implemented as a tests-only plugin now.

Copy link
Contributor

@StarlightIbuki StarlightIbuki left a comment

Choose a reason for hiding this comment

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

I like the idea. It simplifies the test a lot. However, could we rely on this behavior of sequential application of the changes?

spec/helpers.lua Show resolved Hide resolved
spec/helpers.lua Show resolved Hide resolved
@hanshuebner
Copy link
Contributor Author

could we rely on this behavior of sequential application of the changes?

I think the mechanism is reliable enough. It is still that further synchronization is needed, i.e. when testing plugins that perform asynchronous configuration operations, but for most tests, this should be good enough.

@hanshuebner hanshuebner force-pushed the test/reconfiguration-completion-detection-plugin branch from bc43602 to 4af2518 Compare February 21, 2024 14:20
@hanshuebner hanshuebner force-pushed the test/reconfiguration-completion-detection-plugin branch from 4af2518 to a788ec4 Compare February 22, 2024 10:38
Unlike the previous implementation, this one does not require changes
to Kong and its proxy path.  It works based on the assumption that the
order of admin API changes is preserved.  The admin API client marks
the end of the changes that it needs to see propagated to the data
plane(s) by changing the configuration of this plugin, setting a
particular configuration version number.  On the proxy path, a header
X-Kong-Configuration-Version is sent with that version number.  The
plugin's access handler verifies that the version number configured in
the plugin (on the dataplane) matches the version number requested by
the client.  If the version numbers do not match, a 503 error is
generated, which causes the client to retry.

The plugin is available only to busted tests.  It needs to be enabled
when starting Kong.

A new busted test helper function make_synchronized_clients is
provided that automatically synchronizes a proxy client and an admin
API client.  The the test can freely mix invocations to either
endpoints.  Whenever a change is made through the admin API, the proxy
path request is delayed until the change has propagated to the data
plane.  spec/02-integration/13-vaults/06-refresh-secrets_spec.lua has
been updated to use the function as an illustration.
@hanshuebner hanshuebner force-pushed the test/reconfiguration-completion-detection-plugin branch from a788ec4 to eb6f0b6 Compare February 22, 2024 12:45
@hanshuebner hanshuebner merged commit 8a7eac3 into master Feb 22, 2024
31 checks passed
@hanshuebner hanshuebner deleted the test/reconfiguration-completion-detection-plugin branch February 22, 2024 14:14
@hanshuebner hanshuebner added the cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee label Feb 22, 2024
@team-gateway-bot
Copy link
Collaborator

Successfully created cherry-pick PR for master:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee chore Not part of the core functionality of kong, but still needed schema-change-noteworthy size/XL skip-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants