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

Add support for Azure Cache for Redis in all Redis components #3088

Open
berndverst opened this issue Aug 17, 2023 · 17 comments · May be fixed by #3238
Open

Add support for Azure Cache for Redis in all Redis components #3088

berndverst opened this issue Aug 17, 2023 · 17 comments · May be fixed by #3238
Assignees
Labels
kind/enhancement New feature or request P1 pinned Issue does not get stale

Comments

@berndverst
Copy link
Member

This request is similar to the existing Azure Postgres state store support. #2970

AAD auth needs to be supported by fetching an AAD token and using it as the password, the username is the object ID / client ID.

Due to the long lived TCP connections however we need to ensure to manually renew the AAD token in a background goroutine and reinit the connection.

@berndverst berndverst added the kind/enhancement New feature or request label Aug 17, 2023
@berndverst berndverst added this to the v1.13 milestone Aug 17, 2023
@berndverst
Copy link
Member Author

cc @ItalyPaleAle

@ItalyPaleAle
Copy link
Contributor

Due to the long lived TCP connections however we need to ensure to manually renew the AAD token in a background goroutine and reinit the connection.

I don't think this is needed as long as the connection is alive. The token should be refreshed only if the connection needs to be re-established (that's what we do for Postgres and want to do for MySQL)

@shpathak-msft
Copy link

If the token expires, the Azure Cache for Redis server will terminate the connection and it expects a renewed token to keep the connection alive.

@ItalyPaleAle
Copy link
Contributor

If the token expires, the Azure Cache for Redis server will terminate the connection and it expects a renewed token to keep the connection alive.

Since AAD tokens have a lifetime of 1hr, are you saying that connections with Azure Cache for Redis are always terminated every hour forcefully?

(This doesn't seem to be the case for Azure DB for Postgres or Azure SQL AFAICT)

@shpathak-msft
Copy link

shpathak-msft commented Aug 17, 2023

Right, the client applications create long-running, TCP connections to Redis which are expected to be alive for the duration of the application ideally. We typically see connections alive and running for weeks without interruption. So yes, if the redis server does not receive a renewed token before the 1 hour is up, the connection will be terminated.

@ItalyPaleAle
Copy link
Contributor

@shpathak-msft Ok, I understand now.

In practice, does that mean we need to re-send the AUTH command periodically, when we get a new token (like every hours or so)?

Do you know if there's a Go SDK that implements this already?

@shpathak-msft
Copy link

shpathak-msft commented Aug 17, 2023

Looks like go-redis supports AUTH command but you would need to call the AUTH command periodically and supply renewed token.

@berndverst
Copy link
Member Author

@shpathak-msft can you confirm sending the AUTH command periodically will actually work with your system? Or do we completely need to terminate the connection?

@shpathak-msft
Copy link

Sending auth command periodically works. All our code samples and the StackExchange.Redis extension package send auth command periodically. This periodic auth command was the workaround to avoid terminating the connection as closing/creating connections is expensive on the server side and we want to avoid terminating as much as possible.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged (pinned, good first issue, help wanted or triaged/resolved) or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Sep 17, 2023
@ItalyPaleAle ItalyPaleAle added P1 pinned Issue does not get stale and removed stale labels Sep 17, 2023
@dstarkowski
Copy link

Is this issue addressing all Redis components, or just the state store?
I was looking at using Azure AD auth with the lock component.

@sadath-12
Copy link
Contributor

Interesting ! I would like to try it

@sadath-12
Copy link
Contributor

/assign

@sadath-12
Copy link
Contributor

sadath-12 commented Nov 17, 2023

@berndverst @ItalyPaleAle @shpathak-msft , Since we want to refresh token too frequently do we have to always request it from Azure AAD ? or we can use some random token to save that frequent request to cloud ?

@shpathak-msft
Copy link

It needs to be an actual AAD token as it will be validated on the server side.

@sadath-12 sadath-12 linked a pull request Nov 20, 2023 that will close this issue
3 tasks
@berndverst
Copy link
Member Author

berndverst commented Jan 25, 2024

@dstarkowski this issue will impact all Dapr Redis components

@berndverst berndverst changed the title Add support for Azure Cache for Redis in state store Add support for Azure Cache for Redis in all Redis components (except PubSub) Jan 25, 2024
@berndverst berndverst changed the title Add support for Azure Cache for Redis in all Redis components (except PubSub) Add support for Azure Cache for Redis in all Redis components Jan 25, 2024
@berndverst berndverst modified the milestones: v1.13, v1.14 Jan 25, 2024
@berndverst
Copy link
Member Author

To complete Entra ID / Azure AD support in Redis components someone needs to complete the PR #3238 or start from scratch.

The idea is to take the azureidentity library to request an azcore.AzureTokenCredential and call GetToken on this to request the token. (we have shared code for this already in the common/authentication/azure folder which is used by all components). This token needs to be sent to Redis via the Redis client AUTH command. Before the token expires (say 2 minutes before expiry) a new token needs to be requested via GetToken and once against sent via AUTH. This needs to be done in the background until eternity.

@berndverst berndverst removed this from the v1.14 milestone May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request P1 pinned Issue does not get stale
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants