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(acme): support redis namespace #10562

Merged
merged 7 commits into from
Mar 29, 2023

Conversation

catbro666
Copy link
Contributor

@catbro666 catbro666 commented Mar 27, 2023

Summary

This PR relies on fffonion/lua-resty-acme#101

namespace will be treated as a prefix of key and is default to empty string "" for backward compatibility. namespace must not be prefixed with any of the reserved words.

Checklist

Full changelog

  • Acme plugin now supports configuring namespace for redis storage which is default to empty string for backward compatibility.

Issue reference

KAG-615

This PR relies on [fffonion/lua-resty-acme#101](fffonion/lua-resty-acme#101)

`namespace` will be treated as a prefix of key and is default to empty
string `""` for backward compatibility. `namespace` must not be prefixed
with any of the reserverd words.

[KAG-615](https://konghq.atlassian.net/browse/KAG-615)
@catbro666 catbro666 force-pushed the KAG-615-acme-support-redis-namespace branch from f1f266a to 8287f8b Compare March 27, 2023 06:36
@catbro666 catbro666 requested a review from fffonion March 27, 2023 07:30

local CERT_TYPES = { "rsa", "ecc" }

local RSA_KEY_SIZES = { 2048, 3072, 4096 }

local STORAGE_TYPES = { "kong", "shm", "redis", "consul", "vault" }

local function validate_namespace(namespace)
Copy link
Contributor

@fffonion fffonion Mar 27, 2023

Choose a reason for hiding this comment

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

let's make sure to communicate with koko team on this constraint to ensure it doesn't get missed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, will inform the koko team on the JIRA ticket.

CHANGELOG.md Show resolved Hide resolved
kong/plugins/acme/schema.lua Show resolved Hide resolved
Co-authored-by: Chrono <[email protected]>
Copy link
Contributor

@fffonion fffonion left a comment

Choose a reason for hiding this comment

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

merge on green

@hanshuebner hanshuebner merged commit c36fe3e into master Mar 29, 2023
@hanshuebner hanshuebner deleted the KAG-615-acme-support-redis-namespace branch March 29, 2023 15:09
@hbagdi
Copy link
Member

hbagdi commented Mar 29, 2023

@fffonion @hanshuebner @catbro666 Why was this PR merged without a corresponding documentation PR?

@hbagdi
Copy link
Member

hbagdi commented Mar 29, 2023

@catbro666 Please open the corresponding EE PR.

catbro666 added a commit to Kong/docs.konghq.com that referenced this pull request Mar 30, 2023
correspond to [Kong/kong#10562](Kong/kong#10562)

`namespace` will be treated as a prefix of key and is default to empty string `""` for backward compatibility.
`namespace` must not be prefixed with any of the reserved words.

[KAG-615](https://konghq.atlassian.net/browse/KAG-615)
@catbro666
Copy link
Contributor Author

Opened the corresponding EE/DOC PR.

@hbagdi
Copy link
Member

hbagdi commented Mar 31, 2023

@hanshuebner @catbro666 Why was this commit merged without any tests that assert that the namespace parameter is used and takes effect or not?

@catbro666
Copy link
Contributor Author

@hbagdi Because the storage_config is passed directly to the bottom library. There's nothing special about the namespace parameter compared to the other parameters. So I think the current tests can ensure that it takes effect.

@hbagdi
Copy link
Member

hbagdi commented Apr 3, 2023

Is there a test that proves that the configuration parameter is correctly passed down to the library?

@hbagdi
Copy link
Member

hbagdi commented Apr 4, 2023

ping @catbro666

@catbro666
Copy link
Contributor Author

catbro666 commented Apr 4, 2023

@hbagdi Sorry, I may have misunderstood you before.

any tests that assert that the namespace parameter is used and takes effect or not?

I think this test can cover this. It creates 3 namespaces, one default and two specified. They can work independently and don't interfere with each other.

@hbagdi
Copy link
Member

hbagdi commented Apr 4, 2023

That test tests that the library can take namespace as an input and that there is isolation between namespaces.
It doesn't prove that if a user configures a 'namespace' in the acme plugin configuration, the code in this repository will indeed configure the library accordingly.
In other words, there is no test that would fail if the namespace configuration input is ignored by Kong and not passed into the underlying library.

Does that make sense?

@catbro666
Copy link
Contributor Author

catbro666 commented Apr 5, 2023

It doesn't prove that if a user configures a 'namespace' in the acme plugin configuration, the code in this repository will indeed configure the library accordingly.

@hbagdi Indeed, that test can't prove this. But this part is a common logic shared by all storage and independent of the specific storage implementation. The storage configuration is transparent to the plugin, the plugin just simply passes it down to the library. As for proving the storage_config can be successfully passed to the library, we already have corresponding tests in 01-client_spec.lua and 03_access_spec.lua.

In my opinion, if the parts of a system are orthogonal to each other, we can test them individually. If we test everything at the top level, the number of combinations will explode.

@hbagdi
Copy link
Member

hbagdi commented Apr 5, 2023

As for proving the storage_config can be successfully passed to the library, we already have corresponding tests in 01-client_spec.lua and 03_access_spec.lua.

I'm struggling so please bear with me.
I don't see a unit test that ensures that the configuration from the schema is translated into the configuration table passed down to the underlying library correctly.

@catbro666
Copy link
Contributor Author

@hbagdi Added tests in this PR to ensure that the namespace parameter is used and takes effect.

cloudjumpercat added a commit to Kong/docs.konghq.com that referenced this pull request May 2, 2023
* docs(acme): support redis namespace

correspond to [Kong/kong#10562](Kong/kong#10562)

`namespace` will be treated as a prefix of key and is default to empty string `""` for backward compatibility.
`namespace` must not be prefixed with any of the reserved words.

[KAG-615](https://konghq.atlassian.net/browse/KAG-615)

* update

* Fix Vale errors

Signed-off-by: Diana <[email protected]>

* Apply suggestions from code review

---------

Signed-off-by: Diana <[email protected]>
Co-authored-by: Diana <[email protected]>
fabianrbz pushed a commit to Kong/docs.konghq.com that referenced this pull request May 16, 2023
* docs(acme): support redis namespace

correspond to [Kong/kong#10562](Kong/kong#10562)

`namespace` will be treated as a prefix of key and is default to empty string `""` for backward compatibility.
`namespace` must not be prefixed with any of the reserved words.

[KAG-615](https://konghq.atlassian.net/browse/KAG-615)

* update

* Fix Vale errors

Signed-off-by: Diana <[email protected]>

* Apply suggestions from code review

---------

Signed-off-by: Diana <[email protected]>
Co-authored-by: Diana <[email protected]>
lena-larionova added a commit to Kong/docs.konghq.com that referenced this pull request May 19, 2023
* gateway 3.3 metadata

* feat(proxy-cache): add ignore_uri_case to configuring cache-key uri to be handled as lowercase (#5270)

* feat(proxy-cache): add cache_lowercase_uri param

* renaming parameter to ignore_uri_case

* feat(proxy-cache): apply suggestions from code review

Co-authored-by: Diana <[email protected]>

* feat(proxy-cache): removed in proxy-cache-advanced

* feat(proxy-cache): apply suggestions from code review

Co-authored-by: Samuele Illuminati <[email protected]>

* feat(proxy-cache): add param to plugin-ignore

---------

Co-authored-by: Diana <[email protected]>
Co-authored-by: Samuele Illuminati <[email protected]>

* docs(acme): add account_key for acme plugin (#5222)

* docs(acme): add account_key for acme plugin

* docs(acme): add keys/keyset configuration for acme plugin

* Update app/_hub/kong-inc/acme/_index.md

Co-authored-by: lena-larionova <[email protected]>

* Update app/_hub/kong-inc/acme/_index.md

Co-authored-by: lena-larionova <[email protected]>

* Update app/_hub/kong-inc/acme/_index.md

Co-authored-by: lena-larionova <[email protected]>

* Update app/_hub/kong-inc/acme/_index.md

Co-authored-by: lena-larionova <[email protected]>

---------

Co-authored-by: lena-larionova <[email protected]>

* feat(logging): add workspace name in the logging payload (#5251)

Co-authored-by: lena.larionova <[email protected]>

* docs(http-log): plugin schema update for 3.3 (#5326)

* feat(plugins): added second available content-type option to http-log plugin

* add version tags and a changelog entry

---------

Co-authored-by: lena-larionova <[email protected]>

* docs(proxy-cache-advanced): add ignore_uri_case (#5458)

* docs(proxy-cache-advanced): add ignore_uri_case

add ignore_uri_case parameter to proxy-cache-advanced docs

* Update app/_hub/kong-inc/proxy-cache-advanced/_index.md

Co-authored-by: lena-larionova <[email protected]>

* Set minimum version on param

---------

Co-authored-by: lena-larionova <[email protected]>

* docs(request-transformer): untrusted_lua (#5499)

add changelog for untrusted lua to request transformer and request
transformer advanced plugins

* docs(aws-lambda): add new disable_https field (#5486)

* docs(aws-lambda): add new disable_https field

* add changelog entry

* phrasing adjustment for changelog

---------

Co-authored-by: lena-larionova <[email protected]>

* docs(3.3) remove references to 'default tracing_sampling_rate' (#5517)

* docs(acme): support redis namespace (#5366)

* docs(acme): support redis namespace

correspond to [Kong/kong#10562](Kong/kong#10562)

`namespace` will be treated as a prefix of key and is default to empty string `""` for backward compatibility.
`namespace` must not be prefixed with any of the reserved words.

[KAG-615](https://konghq.atlassian.net/browse/KAG-615)

* update

* Fix Vale errors

Signed-off-by: Diana <[email protected]>

* Apply suggestions from code review

---------

Signed-off-by: Diana <[email protected]>
Co-authored-by: Diana <[email protected]>

* feat(proxy-cache): add wildcard and parameter match support for content_type (#5420)

* feat(proxy-cache): add wildcard and parameter match support for content_type

* Apply suggestions from code review

Co-authored-by: Michael Heap <[email protected]>

* set version tags for 3.3; add missing changelog entry for proxy cache

---------

Co-authored-by: lena-larionova <[email protected]>
Co-authored-by: Michael Heap <[email protected]>
Co-authored-by: lena.larionova <[email protected]>

* datadog: now host is referenceable (#5353)

* datadog: now host is referenceable

* Update app/_hub/kong-inc/datadog/_index.md

Co-authored-by: lena-larionova <[email protected]>

* kubernetes section

* Apply suggestions from code review

Co-authored-by: lena-larionova <[email protected]>

* Update app/_hub/kong-inc/datadog/_index.md

Co-authored-by: Harry <[email protected]>

* - fix indents
- add changelog entry
- adjust phrasing for intro to kubernetes section and add navtabs
- set version tags

---------

Co-authored-by: lena-larionova <[email protected]>
Co-authored-by: Victor Yu <[email protected]>
Co-authored-by: Harry <[email protected]>
Co-authored-by: lena.larionova <[email protected]>

* [DOCU-3216] Status Endpoint link (#5541)

* add status endpoint

* add absolute_url

* Fix description of routing priorities with multiple paths (#5533)

* Fix description of routing priorities with multiple paths

* Apply suggestions from code review

---------

Co-authored-by: Diana <[email protected]>

* Revert "feat(logging): add workspace name in the logging payload (#52… (#5547)

Revert "feat(logging): add workspace name in the logging payload (#5251)"

This reverts commit 37df8eb.

* fix(oauth2): add a changelog entry (#5548)

* fix(oauth2): add a changelog entry

* rephrase for clarity

---------

Co-authored-by: lena-larionova <[email protected]>

* [DOCU-2921] Status endpoint + Kong health  check (#5419)

* Create health-check.md

New page + tutorial tempalte for KAG-76

* docs: health check

* Update health-check.md

* Update app/_src/gateway/production/monitoring/health-check.md

Co-authored-by: Chrono <[email protected]>

* Update app/_src/gateway/production/monitoring/health-check.md

Co-authored-by: Chrono <[email protected]>

* Update health-check.md

* Update app/_src/gateway/production/monitoring/health-check.md

Co-authored-by: Chrono <[email protected]>

* Update app/_src/gateway/production/monitoring/health-check.md

Co-authored-by: Datong Sun <[email protected]>

* Update app/_src/gateway/production/monitoring/health-check.md

Co-authored-by: Datong Sun <[email protected]>

* Update app/_src/gateway/production/monitoring/health-check.md

Co-authored-by: Datong Sun <[email protected]>

* Update app/_src/gateway/production/monitoring/health-check.md

Co-authored-by: Datong Sun <[email protected]>

* Update app/_src/gateway/production/monitoring/health-check.md

Co-authored-by: Datong Sun <[email protected]>

* Update app/_src/gateway/production/monitoring/health-check.md

Co-authored-by: Datong Sun <[email protected]>

* Update app/_src/gateway/production/monitoring/health-check.md

Co-authored-by: Datong Sun <[email protected]>

* Update app/_src/gateway/production/monitoring/health-check.md

Co-authored-by: Datong Sun <[email protected]>

* Update app/_src/gateway/production/monitoring/health-check.md

Co-authored-by: Datong Sun <[email protected]>

* Update app/_src/gateway/production/monitoring/health-check.md

Co-authored-by: Datong Sun <[email protected]>

* Update app/_src/gateway/production/monitoring/health-check.md

Co-authored-by: Datong Sun <[email protected]>

* Update app/_src/gateway/production/monitoring/health-check.md

Co-authored-by: Datong Sun <[email protected]>

* Update app/_src/gateway/production/monitoring/health-check.md

Co-authored-by: Datong Sun <[email protected]>

* Update app/_src/gateway/production/monitoring/health-check.md

Co-authored-by: Datong Sun <[email protected]>

* Update app/_src/gateway/production/monitoring/health-check.md

Co-authored-by: Datong Sun <[email protected]>

* Update app/_src/gateway/production/monitoring/health-check.md

Co-authored-by: Datong Sun <[email protected]>

* Change some phrasing

These are all small adjustments.

* Update status_listen enablement step

* s/configured/enabled/ to match other updates

* Add response examples to usage instructions

* Fixup code blocks

* Add health check link to navbar

* Fixup response code/messages

* Use the correct link syntax

* Add reference to monitoring index/overview

* Inline port number comment in yaml block

* Prettify JSON for readability

* Update app/_src/gateway/production/monitoring/index.md

Co-authored-by: Vinicius Mignot <[email protected]>

* readiness instead of health

* Apply suggestions from code review

Co-authored-by: Vinicius Mignot <[email protected]>

* s/ready to work/ready to accept requests/

* add control_plane explanation

* apply review suggestions

* more review comments addressing

* Update app/_src/gateway/production/monitoring/readiness-check.md

Co-authored-by: Harry <[email protected]>

* Revert "Update app/_src/gateway/production/monitoring/readiness-check.md"

This reverts commit d68d904.

* wording and formatting

* Copy edit

* link

---------

Co-authored-by: pluveto <[email protected]>
Co-authored-by: Zijing Zhang <[email protected]>
Co-authored-by: Chrono <[email protected]>
Co-authored-by: Datong Sun <[email protected]>
Co-authored-by: Michael Martin <[email protected]>
Co-authored-by: Michael Martin <[email protected]>
Co-authored-by: Chrono <[email protected]>
Co-authored-by: Vinicius Mignot <[email protected]>
Co-authored-by: Harry <[email protected]>
Co-authored-by: Angel <[email protected]>

* Reworked plugin queues (#5088)

* Reworked plugin queues documentation

 * Add queueing parameters to relevant plugin docs
 * Mention queues in sizing guidelines
 * Add batch queue overview and reference

* Remove consumer batching section, add results sentence to why queues are needed

Signed-off-by: Diana <[email protected]>

* Apply copy edits

Co-authored-by: Hans Hübner <[email protected]>

* Rework how queue parameters are included in plugin docs

* Can't include paramters

* Quote queue parameter names

* remove comment

* remove comment

* Change batch queue to queue, fix links, add link to Zipkin to about plugin queues

Signed-off-by: Diana <[email protected]>

* Fix links to queue reference

Signed-off-by: Diana <[email protected]>

* Apply conditional rendering to queue content

* Add conditional rendering to queue parameters

* Fix minimum_version spacing

Signed-off-by: Diana <[email protected]>

* Fix queue param spacing in http log plugin doc

Signed-off-by: Diana <[email protected]>

* Apply suggestions from code review

Co-authored-by: Diana <[email protected]>
Co-authored-by: Angel <[email protected]>

* Update app/_src/gateway/production/sizing-guidelines.md

Co-authored-by: Angel <[email protected]>

* Fix link and add conditional rendering to sizing guidelines page

---------

Signed-off-by: Diana <[email protected]>
Co-authored-by: Diana <[email protected]>
Co-authored-by: Angel <[email protected]>

* [DOCU-3051] Auto docs 3.3 (#5549)

* autodocs

* Apply suggestions from code review

* config.md add EE info

* Update app/_src/gateway/reference/configuration.md

Co-authored-by: lena-larionova <[email protected]>

---------

Co-authored-by: lena-larionova <[email protected]>

* Timeout support for dynamic log levels (#5550)

* Timeout support for dynamic log levels

KAG-5
DOCU-2903

* Update app/_src/gateway/production/logging/update-log-level-dynamically.md

* Update app/_src/gateway/production/logging/update-log-level-dynamically.md

---------

Co-authored-by: Angel <[email protected]>

* Spec links (#5553)

add links to specs

* chore: update 3.3 nav to include all updates in 3.2

* [DOCU-3065] Data plane labels in konnect (#5554)

* document using custom dp labels in konnect

* Update app/konnect/runtime-manager/runtime-instances/custom-dp-labels.md

---------

Co-authored-by: Angel <[email protected]>

* [DOCU-3167] Add Amazon Linux 2023 to support page (#5536)

* Add Amazon Linux 2023 to support page

Signed-off-by: Diana <[email protected]>

* Add Amazon Linux 2023 tab to install instructions

Signed-off-by: Diana <[email protected]>

* Apply suggestions from code review

Co-authored-by: lena-larionova <[email protected]>

* Add 3.3 to browser and third-party support, add AL2023 Yum repo tab

Signed-off-by: Diana <[email protected]>

* Update app/_data/tables/support/gateway/versions/33.yml

Co-authored-by: lena-larionova <[email protected]>

* Add Amazon Graviton note to install pages

Signed-off-by: Diana <[email protected]>

* Remove mentions of Graviton

Signed-off-by: Diana <[email protected]>

* Fix EOL date for 3.3

Signed-off-by: Diana <[email protected]>

---------

Signed-off-by: Diana <[email protected]>
Co-authored-by: lena-larionova <[email protected]>

* Revert "[DOCU-3167] Add Amazon Linux 2023 to support page" (#5559)

Revert "[DOCU-3167] Add Amazon Linux 2023 to support page (#5536)"

This reverts commit 9c64603.

* Fixes to log level doc (#5570)

fixes

* Support page updates for 3.3 (#5569)

* update support pages for 3.3

* update one more version

Co-authored-by: Angel <[email protected]>

---------

Co-authored-by: Angel <[email protected]>

* Fix errors after rebase

* feat: add aws iam authentication to rds feature document (#5503)

* feat: add aws iam authentication to rds feature document

* refine doc

* add more tips

* ssl maximum version is limited by rds

* add mentioning cluster resource id syntax for aurora

* adjust a bit about IAM role assignment and add the case of running kong locally

* make onboarding smoother by providing ways to connect to rds using psql; refine the wording on read/write and read/only mode description

* mention tlsv1.3 not supported one more time in the limitations; refine wording of the highlight of this feature

* Edit title, introduction, and start of prereqs section

Signed-off-by: Diana <[email protected]>

* Finish copy edits

Signed-off-by: Diana <[email protected]>

* Apply tech review feedback

Signed-off-by: Diana <[email protected]>

* Fix some grammar errors and spelling

Signed-off-by: Diana <[email protected]>

* Apply suggestions from code review

Co-authored-by: Qirui(Keery) Nie <[email protected]>

* Update app/_src/gateway/kong-enterprise/aws-iam-auth-to-rds-database.md

Co-authored-by: Qirui(Keery) Nie <[email protected]>

* Fix limitations descriptions based on tech review feedback

Signed-off-by: Diana <[email protected]>

---------

Signed-off-by: Diana <[email protected]>
Co-authored-by: Diana <[email protected]>

* Plugin queuing diagrams (#5583)

* Add plugin queue diagrams

Signed-off-by: Diana <[email protected]>

* Fix Figure number on second one

Signed-off-by: Diana <[email protected]>

---------

Signed-off-by: Diana <[email protected]>

* add 3.3 compatibility table updates (#5573)

add 3.3 compatability issues

Adds compatability issues for release 3.3.

* Remove centos from 3.3 support table

* [DOCU-3169] Changelog for gateway 3.3.0.0 (#5562)

* changelog for gateway 3.3.0.0

* added reminders about deprecations and links to API specs

* set release date

* [DOCU-3215] 3.3 upgrade guide (#5590)

* Add upgrade table for 3.3

Signed-off-by: Diana <[email protected]>

* Fix rolling upgrades column for 3.1.x releases

* Add info about plugin queue breaking changes to upgrade doc

Signed-off-by: Diana <[email protected]>

* Add traditional_compat and Plugins to breaking changes section

Signed-off-by: Diana <[email protected]>

* Fix changelog link

Signed-off-by: Diana <[email protected]>

* Apply suggestions from code review

Co-authored-by: lena-larionova <[email protected]>

---------

Signed-off-by: Diana <[email protected]>
Co-authored-by: lena-larionova <[email protected]>

* [docu-3186] SOBM page (#5558)

* add page

* Apply suggestions from code review

Co-authored-by: lena-larionova <[email protected]>

* rename

---------

Co-authored-by: lena-larionova <[email protected]>

* [fix] 3.2.2 changelog (#5605)

* fix 3.2.2 changelog

* move content into the right places

---------

Co-authored-by: lena.larionova <[email protected]>

* Add known issues to 3.3 changelog (#5599)

* lmdb known issue

* adding more known issues

* spacing

* Apply suggestions from code review

Co-authored-by: Datong Sun <[email protected]>

---------

Co-authored-by: Angel <[email protected]>
Co-authored-by: Datong Sun <[email protected]>

---------

Signed-off-by: Diana <[email protected]>
Co-authored-by: Tobias Lindberg <[email protected]>
Co-authored-by: Diana <[email protected]>
Co-authored-by: Samuele Illuminati <[email protected]>
Co-authored-by: Steve Zesch <[email protected]>
Co-authored-by: Xiaoyan Rao <[email protected]>
Co-authored-by: Jack Tysoe <[email protected]>
Co-authored-by: Qirui(Keery) Nie <[email protected]>
Co-authored-by: Enrique García Cota <[email protected]>
Co-authored-by: Zhefeng C <[email protected]>
Co-authored-by: Yusheng Li <[email protected]>
Co-authored-by: Michael Heap <[email protected]>
Co-authored-by: Xumin <[email protected]>
Co-authored-by: Victor Yu <[email protected]>
Co-authored-by: Harry <[email protected]>
Co-authored-by: Angel <[email protected]>
Co-authored-by: Hans Hübner <[email protected]>
Co-authored-by: Amy Goldsmith <[email protected]>
Co-authored-by: pluveto <[email protected]>
Co-authored-by: Zijing Zhang <[email protected]>
Co-authored-by: Chrono <[email protected]>
Co-authored-by: Datong Sun <[email protected]>
Co-authored-by: Michael Martin <[email protected]>
Co-authored-by: Michael Martin <[email protected]>
Co-authored-by: Chrono <[email protected]>
Co-authored-by: Vinicius Mignot <[email protected]>
Co-authored-by: Murillo <[email protected]>
Co-authored-by: Fabian Rodriguez <[email protected]>
Co-authored-by: Alex Gaesser <[email protected]>
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.

5 participants