You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flink.apache.org by Alexander Fedulov <al...@gmail.com> on 2023/04/28 12:41:28 UTC

[DISCUSS] SqlClient gateway mode: support for URLs

I would like to discuss the current implementation of the SQL Gateway
support in SQL Cli Client and how it can be improved.

1) *hosname:port/v1 vs
https://hostname:port/flink-clusters/session-cluster-1/v1 *
Currently, the *--endpoint* parameter needs to be specified in the
*InetSocketAddress*  format, i.e. *hostname:port.* While this works fine
for basic use cases, it does not support the placement of the gateway
behind a proxy or using an Ingress for routing to a specific Flink cluster
based on the URL path.  I.e. it expects *some.hostname.com:9001
<http://some.hostname.com:9001>*  to directly serve requests on
*some.hostname.com:9001/v1
<http://some.hostname.com:9001/v1>* . Mapping to a non-root location,
i.e. *some.hostname.com:9001/flink-clusters/sql-preview-cluster-1/v1
<http://some.hostname.com:9001/flink-clusters/sql-preview-cluster-1/v1>*
 is not supported. Since the client talks to the gateway via its REST
endpoint, I believe that the right format for the *--endpoint*  parameter
is *URL*, not *InetSocketAddress* .

2) *HTTPS support*
Another related issue is that internally SQL Client uses  Flink’s
*RestClient* [1].  This client decides whether to enable SSL not on the
basis of the URL schema (https://...), but based on Flink configuration,
namely a global *security.ssl.rest.enabled*  parameter [2] (which is also
used for the REST server-side configuration ). When this parameter is set
to true, it automatically requires user-supplied
*security.ssl.rest.truststore*  and *security.ssl.rest.keystore* to be
configured - there is no default option to use certificates from JDK. I was
wondering if there is any real benefit in handling the low-level Netty
channels and certificates manually for the use case of connecting between
 SQL Cli Client and SQL Gateway REST API.  There is already a dependency on
*OkHttpClient*  in *flink-metrics*. I would like to hear what you think
about switching to *OkHttp* and adding the ability to optionally load
custom certificates there rather than patching *RestClient*.

[1]
https://github.com/apache/flink/blob/5dddc0dba2be20806e67769314eecadf56b87a53/flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/gateway/ExecutorImpl.java#L359
[2]
https://github.com/apache/flink/blob/5d9e63a16f079399c6b51547284bb96db0326bdb/flink-runtime/src/main/java/org/apache/flink/runtime/rest/RestClientConfiguration.java#L103

Best,
Alex Fedulov

Re: [DISCUSS] SqlClient gateway mode: support for URLs

Posted by Alexander Fedulov <al...@gmail.com>.
One more requirement surfaced in the scope of introducing the above
mentioned improvements -  the need for SQL Gateway requests to be
authenticated. I propose to introduce an environmental variable,
FLINK_REST_CLIENT_HEADERS that could contain a list of HTTPS headers (
including authentication cookies) to be passed along with each RestClient
request.

Any objections or alternative suggestions?

Best,
Alex

On Mon, 8 May 2023 at 20:14, Alexander Fedulov <al...@gmail.com>
wrote:

> Hi Thomas,
>
> Thanks for the feedback. I created two separate tickets to track the
> related work:
> https://issues.apache.org/jira/browse/FLINK-32030
> https://issues.apache.org/jira/browse/FLINK-32035
>
> Best,
> Alex Fedulov
>
> On Wed, 3 May 2023 at 20:25, Thomas Weise <th...@apache.org> wrote:
>
>> Hi Alex,
>>
>> Thanks for the investigation and for the ideas on how to improve the SQL
>> gateway REST client.
>>
>> I think the solution could come in 2 parts. Near term the existing client
>> can be patched to support URL mapping and HTTPS based on a standard URL.
>> If
>> I understand your proposal correctly, that can be implemented in a
>> backward
>> compatible manner and w/o introducing additional dependencies.
>>
>> Long term it would make sense to switch to a specialized REST client, or
>> at
>> least the HTTP client that comes with Java 11 (after Flink says bye to JDK
>> 8 support).
>>
>> Do you have a JIRA for this work?
>>
>> Thanks,
>> Thomas
>>
>>
>> On Wed, May 3, 2023 at 11:46 AM Alexander Fedulov <
>> alexander.fedulov@gmail.com> wrote:
>>
>> > CC'ing some people who worked on the original implementation - curious
>> to
>> > hear your thoughts.
>> >
>> > Alex
>> >
>> > On Fri, 28 Apr 2023 at 14:41, Alexander Fedulov <
>> > alexander.fedulov@gmail.com>
>> > wrote:
>> >
>> > > I would like to discuss the current implementation of the SQL Gateway
>> > > support in SQL Cli Client and how it can be improved.
>> > >
>> > > 1) *hosname:port/v1 vs
>> > > https://hostname:port/flink-clusters/session-cluster-1/v1 *
>> > > Currently, the *--endpoint* parameter needs to be specified in the
>> > > *InetSocketAddress*  format, i.e. *hostname:port.* While this works
>> fine
>> > > for basic use cases, it does not support the placement of the gateway
>> > > behind a proxy or using an Ingress for routing to a specific Flink
>> > cluster
>> > > based on the URL path.  I.e. it expects *some.hostname.com:9001
>> > > <http://some.hostname.com:9001>*  to directly serve requests on *
>> > some.hostname.com:9001/v1
>> > > <http://some.hostname.com:9001/v1>* . Mapping to a non-root location,
>> > > i.e. *some.hostname.com:9001/flink-clusters/sql-preview-cluster-1/v1
>> > > <
>> http://some.hostname.com:9001/flink-clusters/sql-preview-cluster-1/v1>*
>> > >  is not supported. Since the client talks to the gateway via its REST
>> > > endpoint, I believe that the right format for the *--endpoint*
>> parameter
>> > > is *URL*, not *InetSocketAddress* .
>> > >
>> > > 2) *HTTPS support*
>> > > Another related issue is that internally SQL Client uses  Flink’s
>> > > *RestClient* [1].  This client decides whether to enable SSL not on
>> the
>> > > basis of the URL schema (https://...), but based on Flink
>> configuration,
>> > > namely a global *security.ssl.rest.enabled*  parameter [2] (which is
>> also
>> > > used for the REST server-side configuration ). When this parameter is
>> set
>> > > to true, it automatically requires user-supplied
>> > > *security.ssl.rest.truststore*  and *security.ssl.rest.keystore* to be
>> > > configured - there is no default option to use certificates from JDK.
>> I
>> > was
>> > > wondering if there is any real benefit in handling the low-level Netty
>> > > channels and certificates manually for the use case of connecting
>> between
>> > >  SQL Cli Client and SQL Gateway REST API.  There is already a
>> dependency
>> > on
>> > > *OkHttpClient*  in *flink-metrics*. I would like to hear what you
>> think
>> > > about switching to *OkHttp* and adding the ability to optionally load
>> > > custom certificates there rather than patching *RestClient*.
>> > >
>> > > [1]
>> > >
>> >
>> https://github.com/apache/flink/blob/5dddc0dba2be20806e67769314eecadf56b87a53/flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/gateway/ExecutorImpl.java#L359
>> > > [2]
>> > >
>> >
>> https://github.com/apache/flink/blob/5d9e63a16f079399c6b51547284bb96db0326bdb/flink-runtime/src/main/java/org/apache/flink/runtime/rest/RestClientConfiguration.java#L103
>> > >
>> > > Best,
>> > > Alex Fedulov
>> > >
>> >
>>
>

Re: [DISCUSS] SqlClient gateway mode: support for URLs

Posted by Alexander Fedulov <al...@gmail.com>.
Hi Thomas,

Thanks for the feedback. I created two separate tickets to track the
related work:
https://issues.apache.org/jira/browse/FLINK-32030
https://issues.apache.org/jira/browse/FLINK-32035

Best,
Alex Fedulov

On Wed, 3 May 2023 at 20:25, Thomas Weise <th...@apache.org> wrote:

> Hi Alex,
>
> Thanks for the investigation and for the ideas on how to improve the SQL
> gateway REST client.
>
> I think the solution could come in 2 parts. Near term the existing client
> can be patched to support URL mapping and HTTPS based on a standard URL. If
> I understand your proposal correctly, that can be implemented in a backward
> compatible manner and w/o introducing additional dependencies.
>
> Long term it would make sense to switch to a specialized REST client, or at
> least the HTTP client that comes with Java 11 (after Flink says bye to JDK
> 8 support).
>
> Do you have a JIRA for this work?
>
> Thanks,
> Thomas
>
>
> On Wed, May 3, 2023 at 11:46 AM Alexander Fedulov <
> alexander.fedulov@gmail.com> wrote:
>
> > CC'ing some people who worked on the original implementation - curious to
> > hear your thoughts.
> >
> > Alex
> >
> > On Fri, 28 Apr 2023 at 14:41, Alexander Fedulov <
> > alexander.fedulov@gmail.com>
> > wrote:
> >
> > > I would like to discuss the current implementation of the SQL Gateway
> > > support in SQL Cli Client and how it can be improved.
> > >
> > > 1) *hosname:port/v1 vs
> > > https://hostname:port/flink-clusters/session-cluster-1/v1 *
> > > Currently, the *--endpoint* parameter needs to be specified in the
> > > *InetSocketAddress*  format, i.e. *hostname:port.* While this works
> fine
> > > for basic use cases, it does not support the placement of the gateway
> > > behind a proxy or using an Ingress for routing to a specific Flink
> > cluster
> > > based on the URL path.  I.e. it expects *some.hostname.com:9001
> > > <http://some.hostname.com:9001>*  to directly serve requests on *
> > some.hostname.com:9001/v1
> > > <http://some.hostname.com:9001/v1>* . Mapping to a non-root location,
> > > i.e. *some.hostname.com:9001/flink-clusters/sql-preview-cluster-1/v1
> > > <http://some.hostname.com:9001/flink-clusters/sql-preview-cluster-1/v1
> >*
> > >  is not supported. Since the client talks to the gateway via its REST
> > > endpoint, I believe that the right format for the *--endpoint*
> parameter
> > > is *URL*, not *InetSocketAddress* .
> > >
> > > 2) *HTTPS support*
> > > Another related issue is that internally SQL Client uses  Flink’s
> > > *RestClient* [1].  This client decides whether to enable SSL not on the
> > > basis of the URL schema (https://...), but based on Flink
> configuration,
> > > namely a global *security.ssl.rest.enabled*  parameter [2] (which is
> also
> > > used for the REST server-side configuration ). When this parameter is
> set
> > > to true, it automatically requires user-supplied
> > > *security.ssl.rest.truststore*  and *security.ssl.rest.keystore* to be
> > > configured - there is no default option to use certificates from JDK. I
> > was
> > > wondering if there is any real benefit in handling the low-level Netty
> > > channels and certificates manually for the use case of connecting
> between
> > >  SQL Cli Client and SQL Gateway REST API.  There is already a
> dependency
> > on
> > > *OkHttpClient*  in *flink-metrics*. I would like to hear what you think
> > > about switching to *OkHttp* and adding the ability to optionally load
> > > custom certificates there rather than patching *RestClient*.
> > >
> > > [1]
> > >
> >
> https://github.com/apache/flink/blob/5dddc0dba2be20806e67769314eecadf56b87a53/flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/gateway/ExecutorImpl.java#L359
> > > [2]
> > >
> >
> https://github.com/apache/flink/blob/5d9e63a16f079399c6b51547284bb96db0326bdb/flink-runtime/src/main/java/org/apache/flink/runtime/rest/RestClientConfiguration.java#L103
> > >
> > > Best,
> > > Alex Fedulov
> > >
> >
>

Re: [DISCUSS] SqlClient gateway mode: support for URLs

Posted by Thomas Weise <th...@apache.org>.
Hi Alex,

Thanks for the investigation and for the ideas on how to improve the SQL
gateway REST client.

I think the solution could come in 2 parts. Near term the existing client
can be patched to support URL mapping and HTTPS based on a standard URL. If
I understand your proposal correctly, that can be implemented in a backward
compatible manner and w/o introducing additional dependencies.

Long term it would make sense to switch to a specialized REST client, or at
least the HTTP client that comes with Java 11 (after Flink says bye to JDK
8 support).

Do you have a JIRA for this work?

Thanks,
Thomas


On Wed, May 3, 2023 at 11:46 AM Alexander Fedulov <
alexander.fedulov@gmail.com> wrote:

> CC'ing some people who worked on the original implementation - curious to
> hear your thoughts.
>
> Alex
>
> On Fri, 28 Apr 2023 at 14:41, Alexander Fedulov <
> alexander.fedulov@gmail.com>
> wrote:
>
> > I would like to discuss the current implementation of the SQL Gateway
> > support in SQL Cli Client and how it can be improved.
> >
> > 1) *hosname:port/v1 vs
> > https://hostname:port/flink-clusters/session-cluster-1/v1 *
> > Currently, the *--endpoint* parameter needs to be specified in the
> > *InetSocketAddress*  format, i.e. *hostname:port.* While this works fine
> > for basic use cases, it does not support the placement of the gateway
> > behind a proxy or using an Ingress for routing to a specific Flink
> cluster
> > based on the URL path.  I.e. it expects *some.hostname.com:9001
> > <http://some.hostname.com:9001>*  to directly serve requests on *
> some.hostname.com:9001/v1
> > <http://some.hostname.com:9001/v1>* . Mapping to a non-root location,
> > i.e. *some.hostname.com:9001/flink-clusters/sql-preview-cluster-1/v1
> > <http://some.hostname.com:9001/flink-clusters/sql-preview-cluster-1/v1>*
> >  is not supported. Since the client talks to the gateway via its REST
> > endpoint, I believe that the right format for the *--endpoint*  parameter
> > is *URL*, not *InetSocketAddress* .
> >
> > 2) *HTTPS support*
> > Another related issue is that internally SQL Client uses  Flink’s
> > *RestClient* [1].  This client decides whether to enable SSL not on the
> > basis of the URL schema (https://...), but based on Flink configuration,
> > namely a global *security.ssl.rest.enabled*  parameter [2] (which is also
> > used for the REST server-side configuration ). When this parameter is set
> > to true, it automatically requires user-supplied
> > *security.ssl.rest.truststore*  and *security.ssl.rest.keystore* to be
> > configured - there is no default option to use certificates from JDK. I
> was
> > wondering if there is any real benefit in handling the low-level Netty
> > channels and certificates manually for the use case of connecting between
> >  SQL Cli Client and SQL Gateway REST API.  There is already a dependency
> on
> > *OkHttpClient*  in *flink-metrics*. I would like to hear what you think
> > about switching to *OkHttp* and adding the ability to optionally load
> > custom certificates there rather than patching *RestClient*.
> >
> > [1]
> >
> https://github.com/apache/flink/blob/5dddc0dba2be20806e67769314eecadf56b87a53/flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/gateway/ExecutorImpl.java#L359
> > [2]
> >
> https://github.com/apache/flink/blob/5d9e63a16f079399c6b51547284bb96db0326bdb/flink-runtime/src/main/java/org/apache/flink/runtime/rest/RestClientConfiguration.java#L103
> >
> > Best,
> > Alex Fedulov
> >
>

Re: [DISCUSS] SqlClient gateway mode: support for URLs

Posted by Alexander Fedulov <al...@gmail.com>.
CC'ing some people who worked on the original implementation - curious to
hear your thoughts.

Alex

On Fri, 28 Apr 2023 at 14:41, Alexander Fedulov <al...@gmail.com>
wrote:

> I would like to discuss the current implementation of the SQL Gateway
> support in SQL Cli Client and how it can be improved.
>
> 1) *hosname:port/v1 vs
> https://hostname:port/flink-clusters/session-cluster-1/v1 *
> Currently, the *--endpoint* parameter needs to be specified in the
> *InetSocketAddress*  format, i.e. *hostname:port.* While this works fine
> for basic use cases, it does not support the placement of the gateway
> behind a proxy or using an Ingress for routing to a specific Flink cluster
> based on the URL path.  I.e. it expects *some.hostname.com:9001
> <http://some.hostname.com:9001>*  to directly serve requests on *some.hostname.com:9001/v1
> <http://some.hostname.com:9001/v1>* . Mapping to a non-root location,
> i.e. *some.hostname.com:9001/flink-clusters/sql-preview-cluster-1/v1
> <http://some.hostname.com:9001/flink-clusters/sql-preview-cluster-1/v1>*
>  is not supported. Since the client talks to the gateway via its REST
> endpoint, I believe that the right format for the *--endpoint*  parameter
> is *URL*, not *InetSocketAddress* .
>
> 2) *HTTPS support*
> Another related issue is that internally SQL Client uses  Flink’s
> *RestClient* [1].  This client decides whether to enable SSL not on the
> basis of the URL schema (https://...), but based on Flink configuration,
> namely a global *security.ssl.rest.enabled*  parameter [2] (which is also
> used for the REST server-side configuration ). When this parameter is set
> to true, it automatically requires user-supplied
> *security.ssl.rest.truststore*  and *security.ssl.rest.keystore* to be
> configured - there is no default option to use certificates from JDK. I was
> wondering if there is any real benefit in handling the low-level Netty
> channels and certificates manually for the use case of connecting between
>  SQL Cli Client and SQL Gateway REST API.  There is already a dependency on
> *OkHttpClient*  in *flink-metrics*. I would like to hear what you think
> about switching to *OkHttp* and adding the ability to optionally load
> custom certificates there rather than patching *RestClient*.
>
> [1]
> https://github.com/apache/flink/blob/5dddc0dba2be20806e67769314eecadf56b87a53/flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/gateway/ExecutorImpl.java#L359
> [2]
> https://github.com/apache/flink/blob/5d9e63a16f079399c6b51547284bb96db0326bdb/flink-runtime/src/main/java/org/apache/flink/runtime/rest/RestClientConfiguration.java#L103
>
> Best,
> Alex Fedulov
>