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(session) forward persistent cookie field #8187

Merged
merged 8 commits into from
Nov 7, 2022
Merged

feat(session) forward persistent cookie field #8187

merged 8 commits into from
Nov 7, 2022

Conversation

tschaume
Copy link
Contributor

Summary

This PR forwards the session.cookie.persistent field from lua-resty-session. See https://github.com/bungle/lua-resty-session#boolean-sessioncookiepersistent

Full changelog

  • Added persistent field to session.lua and schema.lua

mikefero
mikefero reviewed Apr 7, 2022
View reviewed changes
kong/plugins/session/schema.lua
@@ -54,6 +54,7 @@ return {
{ cookie_httponly = { type = "boolean", default = true } },
{ cookie_secure = { type = "boolean", default = true } },
{ cookie_discard = { type = "number", default = 10 } },
{ cookie_persistent = { type = "boolean", default = false } },
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a breaking change for version compatibility with older data planes; updates will need to be made to the compatibility layer to handle these changes ahead of time of before a release.

@mikefero mikefero added the version-compatibility Incompatibility with older data plane versions label Apr 7, 2022
@tschaume tschaume force-pushed the feat/persistent-cookie branch from 93093ac to 9fedbd5 Compare April 7, 2022 20:49
@tschaume
Copy link
Contributor Author

tschaume commented Apr 7, 2022

@mikefero I added an entry for the cookie_persistent field and kong versions older than 2.8.2. Happy to update if that's not the way to go about it.

@mikefero
Copy link
Contributor

@mikefero I added an entry for the cookie_persistent field and kong versions older than 2.8.2. Happy to update if that's not the way to go about it.

Thanks @tschaume; feel free to revert your commit as there are items in the tests that need to be addressed for the addition into the removed_fields.lua. When this feature lands and is targeted for a particular release the compatibility layer will be updated to handle this new field. Appreciate the effort, it will be easier to address when it gets closer to release time and after this feature has been merged into master.

@tschaume tschaume force-pushed the feat/persistent-cookie branch from 61323cf to 7f7e9d8 Compare April 13, 2022 17:13
@tschaume
Copy link
Contributor Author

sounds good, thanks @mikefero. Reverted the commit and rebased.

@guanlan guanlan requested a review from a team as a August 25, 2022 01:38
@hbagdi
Copy link
Member

hbagdi commented Oct 24, 2022

How does this relate to #8937?

@tschaume
Copy link
Contributor Author

@hbagdi #8937 seems to do the same but adds a test for it. Not sure why a new PR was started for it, though.

@hbagdi
Copy link
Member

hbagdi commented Oct 26, 2022

@tschaume Can you rebase, add the relevant test? That will help us merge this.

@tschaume
Copy link
Contributor Author

@hbagdi I rebased and added the tests from #8937

@hbagdi hbagdi requested a review from gszr October 27, 2022 18:12
mayocream
mayocream approved these changes Oct 30, 2022
View reviewed changes
Copy link
Contributor

@mayocream mayocream left a comment

Choose a reason for hiding this comment

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

Missing a changelog entry, but you can add it in a separate PR.

@dndx
Copy link
Member

dndx commented Oct 31, 2022

@mayocream Could you add the changelog entry for this PR? I think it will be forgotten and not done if merged now.

@dndx dndx merged commit a932444 into Kong:master Nov 7, 2022
@hbagdi hbagdi mentioned this pull request Nov 8, 2022
Closed
locao pushed a commit that referenced this pull request Apr 24, 2024
@raoxiaoyan @sumimakito
feat(rbac): added role_source to update rbac user's rbac role while…
4ce30a5 
… the IDP group mapping changes. (#8187)

* feat(oidc): adds `role_source` to support update RBAC user's rbac role while the IDP group mapping was changed.

---------

Co-authored-by: Makito <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mikefero mikefero mikefero left review comments

@bungle bungle bungle approved these changes

@mayocream mayocream mayocream approved these changes

@hanshuebner hanshuebner Awaiting requested review from hanshuebner

@gszr gszr Awaiting requested review from gszr

Assignees

@gszr gszr

Labels
changelog plugins/session size/M version-compatibility Incompatibility with older data plane versions
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@tschaume @mikefero @hbagdi @dndx @bungle @mayocream @fffonion @gszr @onematchfox @hanshuebner