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

Redis session store #115

Merged
merged 16 commits into from Jul 23, 2020
Merged

Redis session store #115

merged 16 commits into from Jul 23, 2020

Conversation

EmBerz
Copy link

@EmBerz EmBerz commented Jul 16, 2020

No description provided.

@istio-testing
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: EmBerz
To complete the pull request process, please assign sawlanipradeep
You can assign the PR to them by writing /assign @sawlanipradeep in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@googlebot
Copy link
Collaborator

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added the cla: no CLA negative status label Jul 16, 2020
@EmBerz
Copy link
Author

EmBerz commented Jul 16, 2020

@googlebot I consent.

1 similar comment
@margocrawf
Copy link

@googlebot I consent.

margocrawf and others added 15 commits July 22, 2020 16:57
@margocrawf @tylerschultz
Redis session store
e1f4bc6 
- not integrated with the oidc filter yet

Signed-off-by: Tyler Schultz <tschultz@pivotal.io>
@tylerschultz @cfryanr
Use redis session store when configured
5fd6677 
- adds BUILD deps so that linking hiredis works

Signed-off-by: Ryan Richard <rrichard@pivotal.io>
@Changdrew
hmget refactor
426eb90 
Signed-off-by: Ryan Richard <rrichard@pivotal.io>
@cfryanr @Changdrew
Fix whitespace in files from oidc and test/.../oidc directories
9a5c0f4 
Signed-off-by: Andrew Chang <anchang@vmware.com>
@Changdrew @cfryanr
Reduce calls to redis a little more
36cc830 
- When calling hmget, also get the session's time added to avoid an extra hget

Signed-off-by: Ryan Richard <rrichard@pivotal.io>
@cfryanr @Changdrew
Use hmset to reduce calls to redis a little more
6e05753 
Signed-off-by: Andrew Chang <anchang@vmware.com>
@Changdrew @cfryanr
RedisWrapper constructor takes the redis uri
b12777f 
- Hide all sw::redis classes from the users of RedisWrapper

Signed-off-by: Ryan Richard <rrichard@pivotal.io>
@cfryanr @Changdrew
RedisWrapper throws exceptions and RedisSessionStore handles when tim…
6456f2b 
…eouts are infinite

Signed-off-by: Andrew Chang <anchang@vmware.com>
@margocrawf @cfryanr
oidc_filter.cc catches exceptions from all SessionStore method calls
2f7ee83 
- Because a RedisSessionStore might throw an exception for any method

Signed-off-by: Ryan Richard <rrichard@pivotal.io>
@cfryanr @margocrawf
Updated doc to mention Redis as it relates to cookie_name_prefix
fd1f7d2 
- Also capitalized Redis and Authservice
- Also regenerated auto-generated README.md

Signed-off-by: Margo Crawford <margaretc@vmware.com>
@cfryanr
Improve log messages for redis session errors
9abbf86 
- So you can tell what the code was trying to accomplish when the error was raised

Signed-off-by: Margo Crawford <margaretc@vmware.com>
@cfryanr
Enable redis connection pooling
2b1f03f 
- Make the pool size the same as the configured number of worker threads,
  which seems like a reasonable first draft for pool size
- Also remove unused redis `hexists` function from RedisWrapper

Signed-off-by: Margo Crawford <margaretc@vmware.com>
@tylerschultz
Retry redis invocations when redis throws closed errors
d7671a8 
- restructure BUILD files so librarys don't depend on things they
shouldn't. redis wrapper no longer depends on session dependencies.

Signed-off-by: Emily Berk <eberk@vmware.com>
@tylerschultz
Add trace logging for redis v in memory
e6a1271 
Signed-off-by: Tyler Schultz <tschultz@pivotal.io>
@margocrawf
Add retry support for RedisIOErrors
31fed1f 
[#173252885]

Signed-off-by: Emily Berk <eberk@vmware.com>
@tylerschultz
Copy link

@googlebot I consent.

@cfryanr
Copy link

cfryanr commented Jul 23, 2020

@googlebot i consent

@googlebot
Copy link
Collaborator

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot removed the cla: no CLA negative status label Jul 23, 2020
@googlebot googlebot added the cla: yes CLA positive status label Jul 23, 2020
@Changdrew
Copy link

@googlebot I consent!

@tylerschultz
Fix lingering build problems after merge
53a0b5d 
[#171417515]
Signed-off-by: Tyler Schultz <tschultz@vmware.com>
@EmBerz EmBerz merged commit 36dd2ca into master Jul 23, 2020
@EmBerz EmBerz deleted the redis_session_store branch July 23, 2020 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BrenoDeMedeiros BrenoDeMedeiros Awaiting requested review from BrenoDeMedeiros

@liminw liminw Awaiting requested review from liminw

Assignees
No one assigned
Labels
cla: yes CLA positive status size/XXL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@EmBerz @istio-testing @googlebot @margocrawf @tylerschultz @cfryanr @Changdrew