Add client option to disable following redirects

[alprusty]

Description

During 412 release as part of OkHttp version upgrade, explicit client retry logic was removed in the event of a 307/308 redirect. This will cause 401: 'Unauthorized' error when trino client is talking to a Trino Cluster that is deployed behind a load balancer(For example AWS ALB) with a redirect mode(e.g. 307/308) routing policy.

Reference: https://github.com/trinodb/trino/commit/4be14018af945684bb28a53e4dcedfe744529bb0

Okhttp by default does follow redirects. However it drops Authorization headers if the redirect is across hosts.

Reference: https://github.com/square/okhttp/blob/okhttp_4.10.x/okhttp/src/main/kotlin/okhttp3/internal/http/RetryAndFollowUpInterceptor.kt#L323-L328

Solution: This PR introduces a flag --disable-follow-redirects to give an option to clients to follow a redirect or not. By default the flag is set to false which means redirects are going to be followed.

Additional context and related issues

Fixes https://github.com/trinodb/trino/issues/21026

Release notes

( ) This is not user-visible or is docs only, and no release notes are required. ( ) Release notes are required. Please propose a release note for me. ( ) Release notes are required, with the following suggested text:

[cla-bot[bot]]

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: alprusty. This is most likely caused by a git client misconfiguration; please make sure to:

1. check if your git client is configured with an email to sign commits git config --list | grep email
2. If not, set it up using git config --global user.email [email protected]
3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

[cla-bot[bot]]

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: alprusty. This is most likely caused by a git client misconfiguration; please make sure to:

1. check if your git client is configured with an email to sign commits git config --list | grep email
2. If not, set it up using git config --global user.email [email protected]
3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

[cla-bot[bot]]

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: alprusty. This is most likely caused by a git client misconfiguration; please make sure to:

1. check if your git client is configured with an email to sign commits git config --list | grep email
2. If not, set it up using git config --global user.email [email protected]
3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

[oneonestar]

We shouldn't make this change until we've a clear consensus on whether to include the Authorization header on redirects. https://github.com/trinodb/trino/issues/21026#issuecomment-1991556960

[cla-bot[bot]]

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: alprusty. This is most likely caused by a git client misconfiguration; please make sure to:

1. check if your git client is configured with an email to sign commits git config --list | grep email
2. If not, set it up using git config --global user.email [email protected]
3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

[mosabua]

Can you submit a CLA @alprusty ? @martint can process it soon after and trigger check here .. but we can work on reviews and more in parallel.

[mosabua]

Also fyi @sajjoseph

[mosabua]

This was discussed with @electrum @dain @martint and others in the Trino Contributor Call from 21 March 2024.

[cla-bot[bot]]

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: alprusty. This is most likely caused by a git client misconfiguration; please make sure to:

1. check if your git client is configured with an email to sign commits git config --list | grep email
2. If not, set it up using git config --global user.email [email protected]
3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

[mosabua]

CLA is signed .. I am working with @alprusty to get configuration fixed

[mosabua]

@cla-bot check

[cla-bot[bot]]

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: alprusty. This is most likely caused by a git client misconfiguration; please make sure to:

1. check if your git client is configured with an email to sign commits git config --list | grep email
2. If not, set it up using git config --global user.email [email protected]
3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

[cla-bot[bot]]

The cla-bot has been summoned, and re-checked this pull request!

[oneonestar]

@alprusty We also need a regression test on the client actually sent the Authorization header after a redirect.

[alprusty]

@oneonestar I took care of all the review comments.

• Made followRedirects default to true instead of false
• Added Network interceptor to OkHttp builder
• Added a test case to verify Authorization header is being pushed on a redirect https://github.com/trinodb/trino/blob/4dffa6b6d645e26581b5992a8a3caff198446883/client/trino-cli/src/test/java/io/trino/cli/TestClientRedirect.java

Please review again 🙏

[sajjoseph]

It will be great if this PR can be split into two.

1. Control followRedirects in okhttp
2. Retain authorization header during redirects

[alprusty]

squashed all commits as one

[github-actions[bot]]

This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua

[mosabua]

We still want to continue with this. Can you rebase @alprusty

And can others chime in if any more feedback is needed.

Hopefully @martint @dain or @electrum can move this to merge