feature: add redis token refresh support

[sadath-12]

Description

Please explain the changes you've made

The changes proposed will support token refresh of redis using auth command every hour and maintains the connection without terminating as before

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #3088

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

• Code compiles correctly
• Created/updated tests
• Extended the documentation / Created issue in the https://github.com/dapr/docs/ repo: dapr/docs#[issue number]

[daixiang0]

Please fix CI.

ping @johnewart @ItalyPaleAle

[sadath-12]

Sure, Thank you @daixiang0

[JoshVanL]

Some comments from me 🙂

[sadath-12]

Push the changes @JoshVanL . In order to make sure that background goroutine is terminated I have used closeCh . As you mentioned in comments about wait group ? do we have to use it here necessarily ? Since after close that goroutine will either listen to the incoming channel and terminate or if it does not listen means it was terminated anyways

[sadath-12]

loop must run every "a little bit less than 1hr" ? why and how ? please

[ItalyPaleAle]

loop must run every "a little bit less than 1hr" ? why and how ? please

So in here the person from the Azure Redis Cache team explained that the service expects a new AUTH command, with a new token, sent every hour at most.

If you run the loop every 1h exactly, you may be sending the updated command too late, maybe even by a few ms, so this would cause the session to expire.

When there's something that must be renewed periodically, the common pattern is to do that a little bit before the deadline. In this case, if the deadline is 1h, maybe do it every 50m. This also leaves us some buffer, so if requesting the token fails, it can be retried 1m later, and we are still < 1h so the session is kept active.

[sadath-12]

loop must run every "a little bit less than 1hr" ? why and how ? please

So in here the person from the Azure Redis Cache team explained that the service expects a new AUTH command, with a new token, sent every hour at most.

If you run the loop every 1h exactly, you may be sending the updated command too late, maybe even by a few ms, so this would cause the session to expire.

When there's something that must be renewed periodically, the common pattern is to do that a little bit before the deadline. In this case, if the deadline is 1h, maybe do it every 50m. This also leaves us some buffer, so if requesting the token fails, it can be retried 1m later, and we are still < 1h so the session is kept active.

Thank you 😊👌

[sadath-12]

thank you @ItalyPaleAle , following that

[berndverst]

@sadath-12 you need to run your changed files through an autoformatter. gofumpt

[DeepanshuA]

I did following to verify this:

1. Enabled Managed identity on my already existing AKS cluster: az aks update -g -n --enable-managed-identity
2. Created "Azure cache for Redis" - Here, Enabled "Managed Identity"
3. In Authentication tab of my Redis (Azure Portal), I provided my Service Principal (my AKS cluster ObjectId)
4. Created new dapr Build with PR 3238 and deployed that on my AKS cluster
5. Created a new redis component:

kind: Component
metadata:
  name: statestore
spec:
  type: state.redis
  version: v1
  metadata:
  - name: useAzureAD
    value: true

6. Applied nodeapp from hello-kubernetes as it would use redis statestore, but it fails with this error:

time="2024-01-29T12:50:50.191942482Z" level=fatal msg="Fatal error from runtime: process component statestore error: [INIT_COMPONENT_FAILURE]: initialization error occurred for statestore (state.redis/v1): [INIT_COMPONENT_FAILURE]: initialization error occurred for statestore (state.redis/v1): redis store: error connecting to redis at : dial tcp [::1]:6379: connect: connection refused" app_id=nodeapp instance=nodeapp-577cc79c45-4b7rs scope=dapr.runtime type=log ver=edge

When I had tried to add principalId in user identity section of "azure redis", then it was not able to recognize that.
So, I had added to Authentication.

Do we need to enable managed identity some other way to verify this? @shpathak-msft if you could please confirm here?

[github-actions]

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

[berndverst]

I did following to verify this:

1. Enabled Managed identity on my already existing AKS cluster: az aks update -g -n --enable-managed-identity
2. Created "Azure cache for Redis" - Here, Enabled "Managed Identity"
3. In Authentication tab of my Redis (Azure Portal), I provided my Service Principal (my AKS cluster ObjectId)
4. Created new dapr Build with PR 3238 and deployed that on my AKS cluster
5. Created a new redis component:

kind: Component
metadata:
  name: statestore
spec:
  type: state.redis
  version: v1
  metadata:
  - name: useAzureAD
    value: true

6. Applied nodeapp from hello-kubernetes as it would use redis statestore, but it fails with this error:

time="2024-01-29T12:50:50.191942482Z" level=fatal msg="Fatal error from runtime: process component statestore error: [INIT_COMPONENT_FAILURE]: initialization error occurred for statestore (state.redis/v1): [INIT_COMPONENT_FAILURE]: initialization error occurred for statestore (state.redis/v1): redis store: error connecting to redis at : dial tcp [::1]:6379: connect: connection refused" app_id=nodeapp instance=nodeapp-577cc79c45-4b7rs scope=dapr.runtime type=log ver=edge

When I had tried to add principalId in user identity section of "azure redis", then it was not able to recognize that. So, I had added to Authentication.

Do we need to enable managed identity some other way to verify this? @shpathak-msft if you could please confirm here?

@DeepanshuA please use Azure Workload Identity instead. For this you first create a managed identity - user assigned or system assigned. Obviously you need to give the correct Redis permissions (role assignment) to this identity.

Then you must follow these steps for your Dapr enabled deployment:
https://learn.microsoft.com/en-us/azure/aks/workload-identity-overview?tabs=dotnet

azure.workload.identity/use: true
and
azure.workload.identity/client-id which must be set to the client ID of your managed identity you wish to use.

If the AKS cluster has workload identity enabled what will happen is that a special service account token gets mounted in each container of the annotated deployment and env vars are also injected. This allows the Azure Identity SDK to then make a request to authenticate has the managed identity.

I suggest first making sure with another Azure Dapr component that you have workload identity working correctly if you run into issues.

[berndverst]

Apparently we do not have to do this every hour -- instead we should examine the most recent token expiration / duration, and then refresh the token say 10 minutes before it expires.

The token validity is variable and can be longer than one hour.

There should be a function you can use to examine the validity of the token.

[shpathak-msft]

@sadath-12 and @ItalyPaleAle apologies for the confusion in the other issue, adding some clarifications here. Redis server does not care what is the lifetime of the token - it can be 1 hour or 24 hours. The only expectation is that the redis server receives an auth command before the previous token expires. I would recommend not hard coding the "re-auth" loop to run every 50 minutes but it should run ~10 minutes before the token expiry.

[github-actions]

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

[github-actions]

This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

[mikeee]

@sadath-12 This PR is being tracked for v1.14, a few comments are outstanding - do you have the bandwidth to pick this up still?

[berndverst]

I would like to see this exit condition instead be used in the v8client and v9client only.

if s.useAzureAd {
  authenticateWithAzureRedis()
}

or something like that

[berndverst]

I'd rename this to

Suggested change

	func (s *Settings) refreshTokenRoutineForRedis(ctx context.Context, redisClient RedisClient, version string, meta map[string]string, logger logger.Logger) {
	func (s *Settings) authenticateWithAzureRedis(ctx context.Context, redisClient RedisClient, version string, meta map[string]string, logger logger.Logger) {

[berndverst]

The ticker value must be reset every time you get a new token. The value must be token expiration time minus 10 minutes (so that we refresh 10 minutes before expiration).

[berndverst]

No, this is bad.

Instead the Auth method should be added to the interface in redis.go, then an implementation should be added for the v8client.go and v9client.go

That way you can then simply call client.Auth without using the Pipeline client and without using version specific clients in this code. The version should not be required here.

References:
https://pkg.go.dev/github.com/go-redis/redis/v8#Conn.Auth
https://pkg.go.dev/github.com/go-redis/redis/v9#Conn.Auth

[berndverst]

@ItalyPaleAle does this dapr.io/oss-rdbms environment setting always exist? I don't think so?