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

DOC-4819 added Lettuce AMR example #1169

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

andy-stark-redis
Copy link
Contributor

@andy-stark-redis andy-stark-redis commented Feb 14, 2025

DOC-4819

I'm assuming this can basically be the same as the Jedis page except for the different connection example code at the end.

I've attempted to figure out the example by looking at the source code, but I haven't been able to run it, so it probably contains some mistakes :-) Also, I'm not sure if the async test command at the end is overkill for this.

Copy link
Contributor

@andy-stark-redis andy-stark-redis requested review from uglide and a team February 14, 2025 11:28
Copy link
Collaborator

@dwdougherty dwdougherty left a comment

Choose a reason for hiding this comment

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

Apart from a bit of dèjá vu (glitch in the matrix), just one nitpick. Otherwise, LGTM.

@andy-stark-redis
Copy link
Contributor Author

Apart from a bit of dèjá vu (glitch in the matrix), just one nitpick. Otherwise, LGTM.

Thanks @dwdougherty ! Yeah, it's almost the same as the Jedis example, except for the connection bit at the end :-)

@uglide
Copy link
Contributor

uglide commented Feb 17, 2025

@andy-stark-redis Here is a useful doc redis/lettuce#3166

@uglide
Copy link
Contributor

uglide commented Feb 17, 2025

CC: @ggivo

Comment on lines 70 to 71
- `scopes()`: A set of strings defining the [scopes](https://learn.microsoft.com/en-us/entra/identity-platform/scopes-oidc)
you want to apply.
Copy link

Choose a reason for hiding this comment

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

According to "Use Microsoft Entra for cache authentication" expected values for scopes when connecting to Redis are :

Configure your client application to acquire a Microsoft Entra token for scope, https://redis.azure.com/.default or acca5fbb-b7e4-4009-81f1-37e38fd66d78/.default

Probably we can add this as hint to the user so he is not wondering what to provide.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


When you have created your `TokenAuthConfig` instance, you are ready to
connect to AMR.
The example below shows how to include the `TokenAuthConfig` details in a
Copy link

Choose a reason for hiding this comment

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

In the case of Lettuce it is important to point out that lifecycle of TokenBasedRedisCredentialsProvider is not managed by the Lettuce client.

For reference you can check Create Entra ID enabled credentials provider

Create Entra ID enabled credentials provider
The lifecycle of the credentials provider is not managed by the Lettuce client. You can create it once and reuse it across multiple clients\connections. When no longer needed, you should close the provider to release resources TokenBasedRedisCredentialsProvider#close.

Copy link

Choose a reason for hiding this comment

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

I think it will be also useful to provide example code to verify Credentials provider is properly configured

`You can test the credentials provider by obtaining a token.

// Test Entra ID credentials provider can resolve credentials
credentialsSP.resolveCredentials()
.doOnNext(c-> System.out.println(c.getUsername()))
.block();`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Comment on lines 134 to 166
```java
TokenAuthConfig authConfig = EntraIDTokenAuthConfigBuilder.builder()
// Chain of options...
.build();

TokenBasedRedisCredentialsProvider credentialsProvider =
TokenBasedRedisCredentialsProvider.create(tokenAuthConfig);

RedisURI uri = RedisURI.Builder.redis("<host>", <port>)
.withAuthentication(credentialsProvider)
.withSsl(true)
.build();

RedisClient client = RedisClient.create(uri);

SslOptions sslOptions = SslOptions.builder().jdkSslProvider()
.truststore(new File(
"<path_to_truststore.jks_file>"),
"<password_for_truststore.jks_file>"
)
.build();

client.setOptions(ClientOptions.builder()
.sslOptions(sslOptions)
.build());

StatefulRedisConnection<String, String> connection = client.connect();
RedisAsyncCommands<String, String> asyncCommands = connection.async();

// Test the connection.
CompletableFuture<Void> testDBSize = asyncCommands.dbsize()
.thenAccept(r -> {
System.out.println(String.format("Database size: %d", r));
Copy link

Choose a reason for hiding this comment

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

For the complete .withSsl(true) should be sufficient and no additional SslOptions should be needed (in straight forward case when hostname is used )

I can suggest something like

        // Entra ID enabled credentials provider for Service Principle Identity with Client Secret
        TokenBasedRedisCredentialsProvider credentialsSP;
        try (EntraIDTokenAuthConfigBuilder builder = EntraIDTokenAuthConfigBuilder.builder()) {
            builder.clientId(CLIENT_ID).secret(CLIENT_SECRET).authority(AUTHORITY) // "https://login.microsoftonline.com/{YOUR_TENANT_ID}";
                    .scopes(SCOPES); // "https://redis.azure.com/.default";
            credentialsSP = TokenBasedRedisCredentialsProvider.create(builder.build());
        }

        // Test Entra ID credentials provider can resolve credentials
        // credentialsSP.resolveCredentials().doOnNext(c -> System.out.println("SPI ID :" + c.getUsername())).block();

        // Enable automatic re-authentication
        ClientOptions clientOptions = ClientOptions.builder()
                .reauthenticateBehavior(ClientOptions.ReauthenticateBehavior.ON_NEW_CREDENTIALS).build();

        // Use Entra ID credentials provider
        RedisURI redisURI = RedisURI.builder()
                .withHost(HOST)
                .withPort(PORT)
                .withAuthentication(credentialsSP)
                .withSsl(true)
                .build();

        // RedisClient
        RedisClient redisClient = RedisClient.create(redisURI);
        redisClient.setOptions(clientOptions);

        try {
            redisClient.setOptions(clientOptions);
            // Connect with Entra ID credentials provider
            try (StatefulRedisConnection<String, String> user1 = redisClient.connect(StringCodec.UTF8)) {
                System.out.println("Connected to redis as :" + user1.sync().aclWhoami());
                System.out.println("Db size :" + user1.sync().dbsize());
            }
        } finally {
            redisClient.shutdown();  // Shutdown Redis client and close connections
            credentialsSP.close(); // Shutdown Entra ID Credentials provider
        }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link

@ggivo ggivo left a comment

Choose a reason for hiding this comment

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

LGTM

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.

4 participants