Re: RFC - Client side configuration for a SNI proxy
As Bill pointed out, I'm looking into whether we can make an API where a
user could plug in their own proxy implementation, somewhat along the lines
of what Jake suggested. Just to be clear - Jake's code is more of a
demonstration of some concepts than a working prototype. The API we
actually need to be able to plug in a SNI proxy and work well with the rest
of our SSL configuration options is more complicated.
Please stay tuned for a forthcoming proposal, this one is closed.
-Dan
On Thu, Mar 12, 2020 at 3:20 PM Jacob Barrett wrote:
> Everyone,
>
> I would like to hear from more than us 5 people on this. It would be
> really great if others would chime in.
>
> -Jake
>
>
>
> > On Mar 12, 2020, at 3:05 PM, Bill Burcham
> wrote:
> >
> > Sure Udo. Dan is exploring some ideas now.
> >
> > All: let's consider this round of RFC closed (since it officially closed
> > yesterday) and we'll re-open it with a new deadline when we have those
> > changes in hand.
> >
> > On Thu, Mar 12, 2020 at 8:26 AM Udo Kohlmeyer wrote:
> >
> >> Hi there Jake,
> >>
> >> Another twist to the story, but with a working (if unpolished ;) )
> >> prototype.
> >>
> >> It covers all bases of:
> >>
> >> * Type safety
> >> * Extensibility
> >> * Simple API design
> >> * API clarity
> >>
> >> It takes the best of all approaches.
> >>
> >> I like it!!
> >>
> >> +1 to this implementation.
> >>
> >> -1 to Bill's approach.
> >>
> >> @Bill, could we amend the RFC to favor this approach?
> >>
> >> --Udo
> >>
> >> On 3/11/20 11:30 PM, Jacob Barrett wrote:
> >>> -1
> >>>
> >>> I hate to do this but I really feel like we went backwards on this
> >> change.
> >>>
> On Mar 11, 2020, at 3:03 PM, Bill Burcham
> >> wrote:
> PoolFactory {
> setProxyAddress(String host, int port);
> }
>
> ClientCacheFactory {
> setPoolProxyAddress(String host, int port);
> }
> >>> It gives the user no information about what type of proxy is in use. So
> >> documentation must specify that it is only SNI. It assumes that SNI and
> any
> >> other proxy would have a host and port only.
> >>>
> >>> Also consider if we used SRV records to discover the proxy, would I
> need
> >> to set hostname to null or the service name, what about port since it
> comes
> >> from the SRV record. Would I set port to 0?.
> >>>
> >>> What about default ports for well know proxy services like SOCKS?
> Again,
> >> 0?
> >>>
> >>>
> This should address Johns desire for an API that doesn't grow with
> each
> >> new
> proxy type. And it should address Udo's desire for extensibility.
> >>> I really struggle to see how this satisfies future extensibility given
> >> the parameter lock in. Providing a class allows the proxy to define all
> its
> >> options. Consider SniProxyConfig that takes no parameters, it could use
> the
> >> SRV records for “_sniproxy._tcp” queried on the current domain to
> discover
> >> the proxy. Or if given just a hostname could SRV query
> >> “_sniproxy._tcp.hostname” and fall back to A “hostname” on the default
> port.
> >>>
> This API sets the stage for a potential follow-on RFC to support other
> proxy types. One way that might work would be addition of
> setProxyType(String type)/setPoolProxyType(String type) methods. That
> >> RFC
> might reserve the "SNI" proxy type. That RFC might specify an SPI (per
> Jacob's proposal) that would let Geode users register their own proxy
> >> types
> e.g. SOCKS.
> >>> Adding disconnected method to set the proxy type by string doesn’t sit
> >> well for me. What strings are valid? When we add this method what does
> it
> >> mean if we don’t set it, its always SNI?
> >>>
> >>> How would you deprecate a proxy? If the proxy configuration is a class
> I
> >> can discover it in my IDE and deprecate the class when not supported
> >> anymore. Even the original Enum idea could have deprecated enum values.
> >>>
> >>> I would vote for the original RFC over this given that it weakens the
> >> type safety of the API.
> >>>
> >>> To illustrate my point on the SPI see
> >>
> https://github.com/apache/geode/compare/develop...pivotal-jbarrett:wip/proxy-spi
> >> <
> >>
> https://github.com/apache/geode/compare/develop...pivotal-jbarrett:wip/proxy-spi
> >.
> >> Its a quick hack and some things might not be perfect, like discovering
> the
> >> factories, but it should get the idea across.
> >>>
> >>>
> >>> -Jake
> >>>
> >>>
> >>
>
>
Re: RFC - Client side configuration for a SNI proxy
I’m not nearly as familiar with proxies as y’all are, but thinking OO here, I
would expect an API (interface) that covers all interactions that might involve
a proxy, e.g.:
interface Proxy {
Socket connectSocket(InetSocketAddress hostAndPort);
}
You could then have several implementations, such as NoProxy (just creates a
standard socket), SNIProxy, Socks5Proxy, etc, perhaps even a DebugProxy for use
in tests or troubleshooting.
Since each Proxy implementation would be entirely self-contained and properly
initialized by its own appropriate constructors, the Geode API could simply
take a Proxy (no need to think about or constrain what future proxies might
someday be supported, as long as the interface points can be reasonably
anticipated).
The argument made in this thread that “it is hard for the user to discover the
different Proxy subclasses and know what is supported” puzzles me. All IDEs
have a quick way to see implementing classes of an interface. Javadocs also
show that information. Naming convention makes it pretty obvious too, e.g.
class Socks5Proxy implements Proxy. Even just grep'ing for “implements Proxy”
would work. The second part “know what is supported” does not need to be
complicated: if the Proxy interface fully defines the contract for all
interactions that must occur between Geode and any arbitrary Proxy
implementation, then the answer should be very simple: anything that implements
Proxy is a supported proxy implementation…
Maybe there are domain-specific reasons to hardcode support for a particular
proxy rather than enforce separation-of-concerns? I haven’t seen any
discussion on this thread even mentioning encapsulation or the potential
difficulty of doing it properly.
I do agree with Jake’s -1 to the Mar 11 iteration of this proposal. If you
really want to use a Builder pattern to constrain what proxies are permitted
for use with Geode, use a specialized builder for each supported proxy type.
Not all proxies are going to take the exact same parameters (NoProxy needs
none, while some other proxy might require additional certs or auths).
Glad to see this is being discussed. Many of Geode’s other APIs probably could
have benefitted from such a community review ;)
> On Mar 12, 2020, at 3:20 PM, Jacob Barrett wrote:
>
> Everyone,
>
> I would like to hear from more than us 5 people on this. It would be really
> great if others would chime in.
>
> -Jake
>
>
>
>> On Mar 12, 2020, at 3:05 PM, Bill Burcham wrote:
>>
>> Sure Udo. Dan is exploring some ideas now.
>>
>> All: let's consider this round of RFC closed (since it officially closed
>> yesterday) and we'll re-open it with a new deadline when we have those
>> changes in hand.
>>
>> On Thu, Mar 12, 2020 at 8:26 AM Udo Kohlmeyer wrote:
>>
>>> Hi there Jake,
>>>
>>> Another twist to the story, but with a working (if unpolished ;) )
>>> prototype.
>>>
>>> It covers all bases of:
>>>
>>> * Type safety
>>> * Extensibility
>>> * Simple API design
>>> * API clarity
>>>
>>> It takes the best of all approaches.
>>>
>>> I like it!!
>>>
>>> +1 to this implementation.
>>>
>>> -1 to Bill's approach.
>>>
>>> @Bill, could we amend the RFC to favor this approach?
>>>
>>> --Udo
>>>
>>> On 3/11/20 11:30 PM, Jacob Barrett wrote:
-1
I hate to do this but I really feel like we went backwards on this
>>> change.
> On Mar 11, 2020, at 3:03 PM, Bill Burcham
>>> wrote:
> PoolFactory {
> setProxyAddress(String host, int port);
> }
>
> ClientCacheFactory {
> setPoolProxyAddress(String host, int port);
> }
It gives the user no information about what type of proxy is in use. So
>>> documentation must specify that it is only SNI. It assumes that SNI and any
>>> other proxy would have a host and port only.
Also consider if we used SRV records to discover the proxy, would I need
>>> to set hostname to null or the service name, what about port since it comes
>>> from the SRV record. Would I set port to 0?.
What about default ports for well know proxy services like SOCKS? Again,
>>> 0?
> This should address Johns desire for an API that doesn't grow with each
>>> new
> proxy type. And it should address Udo's desire for extensibility.
I really struggle to see how this satisfies future extensibility given
>>> the parameter lock in. Providing a class allows the proxy to define all its
>>> options. Consider SniProxyConfig that takes no parameters, it could use the
>>> SRV records for “_sniproxy._tcp” queried on the current domain to discover
>>> the proxy. Or if given just a hostname could SRV query
>>> “_sniproxy._tcp.hostname” and fall back to A “hostname” on the default port.
> This API sets the stage for a potential follow-on RFC to support other
> proxy types. One way that might work would be addition of
> setProxyType(String type)/setPoolProxyType(String type) methods. That
>>>
Re: RFC - Client side configuration for a SNI proxy
Everyone,
I would like to hear from more than us 5 people on this. It would be really
great if others would chime in.
-Jake
> On Mar 12, 2020, at 3:05 PM, Bill Burcham wrote:
>
> Sure Udo. Dan is exploring some ideas now.
>
> All: let's consider this round of RFC closed (since it officially closed
> yesterday) and we'll re-open it with a new deadline when we have those
> changes in hand.
>
> On Thu, Mar 12, 2020 at 8:26 AM Udo Kohlmeyer wrote:
>
>> Hi there Jake,
>>
>> Another twist to the story, but with a working (if unpolished ;) )
>> prototype.
>>
>> It covers all bases of:
>>
>> * Type safety
>> * Extensibility
>> * Simple API design
>> * API clarity
>>
>> It takes the best of all approaches.
>>
>> I like it!!
>>
>> +1 to this implementation.
>>
>> -1 to Bill's approach.
>>
>> @Bill, could we amend the RFC to favor this approach?
>>
>> --Udo
>>
>> On 3/11/20 11:30 PM, Jacob Barrett wrote:
>>> -1
>>>
>>> I hate to do this but I really feel like we went backwards on this
>> change.
>>>
On Mar 11, 2020, at 3:03 PM, Bill Burcham
>> wrote:
PoolFactory {
setProxyAddress(String host, int port);
}
ClientCacheFactory {
setPoolProxyAddress(String host, int port);
}
>>> It gives the user no information about what type of proxy is in use. So
>> documentation must specify that it is only SNI. It assumes that SNI and any
>> other proxy would have a host and port only.
>>>
>>> Also consider if we used SRV records to discover the proxy, would I need
>> to set hostname to null or the service name, what about port since it comes
>> from the SRV record. Would I set port to 0?.
>>>
>>> What about default ports for well know proxy services like SOCKS? Again,
>> 0?
>>>
>>>
This should address Johns desire for an API that doesn't grow with each
>> new
proxy type. And it should address Udo's desire for extensibility.
>>> I really struggle to see how this satisfies future extensibility given
>> the parameter lock in. Providing a class allows the proxy to define all its
>> options. Consider SniProxyConfig that takes no parameters, it could use the
>> SRV records for “_sniproxy._tcp” queried on the current domain to discover
>> the proxy. Or if given just a hostname could SRV query
>> “_sniproxy._tcp.hostname” and fall back to A “hostname” on the default port.
>>>
This API sets the stage for a potential follow-on RFC to support other
proxy types. One way that might work would be addition of
setProxyType(String type)/setPoolProxyType(String type) methods. That
>> RFC
might reserve the "SNI" proxy type. That RFC might specify an SPI (per
Jacob's proposal) that would let Geode users register their own proxy
>> types
e.g. SOCKS.
>>> Adding disconnected method to set the proxy type by string doesn’t sit
>> well for me. What strings are valid? When we add this method what does it
>> mean if we don’t set it, its always SNI?
>>>
>>> How would you deprecate a proxy? If the proxy configuration is a class I
>> can discover it in my IDE and deprecate the class when not supported
>> anymore. Even the original Enum idea could have deprecated enum values.
>>>
>>> I would vote for the original RFC over this given that it weakens the
>> type safety of the API.
>>>
>>> To illustrate my point on the SPI see
>> https://github.com/apache/geode/compare/develop...pivotal-jbarrett:wip/proxy-spi
>> <
>> https://github.com/apache/geode/compare/develop...pivotal-jbarrett:wip/proxy-spi>.
>> Its a quick hack and some things might not be perfect, like discovering the
>> factories, but it should get the idea across.
>>>
>>>
>>> -Jake
>>>
>>>
>>
Re: RFC - Client side configuration for a SNI proxy
Sure Udo. Dan is exploring some ideas now.
All: let's consider this round of RFC closed (since it officially closed
yesterday) and we'll re-open it with a new deadline when we have those
changes in hand.
On Thu, Mar 12, 2020 at 8:26 AM Udo Kohlmeyer wrote:
> Hi there Jake,
>
> Another twist to the story, but with a working (if unpolished ;) )
> prototype.
>
> It covers all bases of:
>
> * Type safety
> * Extensibility
> * Simple API design
> * API clarity
>
> It takes the best of all approaches.
>
> I like it!!
>
> +1 to this implementation.
>
> -1 to Bill's approach.
>
> @Bill, could we amend the RFC to favor this approach?
>
> --Udo
>
> On 3/11/20 11:30 PM, Jacob Barrett wrote:
> > -1
> >
> > I hate to do this but I really feel like we went backwards on this
> change.
> >
> >> On Mar 11, 2020, at 3:03 PM, Bill Burcham
> wrote:
> >> PoolFactory {
> >> setProxyAddress(String host, int port);
> >> }
> >>
> >> ClientCacheFactory {
> >> setPoolProxyAddress(String host, int port);
> >> }
> > It gives the user no information about what type of proxy is in use. So
> documentation must specify that it is only SNI. It assumes that SNI and any
> other proxy would have a host and port only.
> >
> > Also consider if we used SRV records to discover the proxy, would I need
> to set hostname to null or the service name, what about port since it comes
> from the SRV record. Would I set port to 0?.
> >
> > What about default ports for well know proxy services like SOCKS? Again,
> 0?
> >
> >
> >> This should address Johns desire for an API that doesn't grow with each
> new
> >> proxy type. And it should address Udo's desire for extensibility.
> > I really struggle to see how this satisfies future extensibility given
> the parameter lock in. Providing a class allows the proxy to define all its
> options. Consider SniProxyConfig that takes no parameters, it could use the
> SRV records for “_sniproxy._tcp” queried on the current domain to discover
> the proxy. Or if given just a hostname could SRV query
> “_sniproxy._tcp.hostname” and fall back to A “hostname” on the default port.
> >
> >> This API sets the stage for a potential follow-on RFC to support other
> >> proxy types. One way that might work would be addition of
> >> setProxyType(String type)/setPoolProxyType(String type) methods. That
> RFC
> >> might reserve the "SNI" proxy type. That RFC might specify an SPI (per
> >> Jacob's proposal) that would let Geode users register their own proxy
> types
> >> e.g. SOCKS.
> > Adding disconnected method to set the proxy type by string doesn’t sit
> well for me. What strings are valid? When we add this method what does it
> mean if we don’t set it, its always SNI?
> >
> > How would you deprecate a proxy? If the proxy configuration is a class I
> can discover it in my IDE and deprecate the class when not supported
> anymore. Even the original Enum idea could have deprecated enum values.
> >
> > I would vote for the original RFC over this given that it weakens the
> type safety of the API.
> >
> > To illustrate my point on the SPI see
> https://github.com/apache/geode/compare/develop...pivotal-jbarrett:wip/proxy-spi
> <
> https://github.com/apache/geode/compare/develop...pivotal-jbarrett:wip/proxy-spi>.
> Its a quick hack and some things might not be perfect, like discovering the
> factories, but it should get the idea across.
> >
> >
> > -Jake
> >
> >
>
Re: RFC - Client side configuration for a SNI proxy
Hi there Jake,
Another twist to the story, but with a working (if unpolished ;) )
prototype.
It covers all bases of:
* Type safety
* Extensibility
* Simple API design
* API clarity
It takes the best of all approaches.
I like it!!
+1 to this implementation.
-1 to Bill's approach.
@Bill, could we amend the RFC to favor this approach?
--Udo
On 3/11/20 11:30 PM, Jacob Barrett wrote:
-1
I hate to do this but I really feel like we went backwards on this change.
On Mar 11, 2020, at 3:03 PM, Bill Burcham wrote:
PoolFactory {
setProxyAddress(String host, int port);
}
ClientCacheFactory {
setPoolProxyAddress(String host, int port);
}
It gives the user no information about what type of proxy is in use. So
documentation must specify that it is only SNI. It assumes that SNI and any
other proxy would have a host and port only.
Also consider if we used SRV records to discover the proxy, would I need to set
hostname to null or the service name, what about port since it comes from the
SRV record. Would I set port to 0?.
What about default ports for well know proxy services like SOCKS? Again, 0?
This should address Johns desire for an API that doesn't grow with each new
proxy type. And it should address Udo's desire for extensibility.
I really struggle to see how this satisfies future extensibility given the
parameter lock in. Providing a class allows the proxy to define all its
options. Consider SniProxyConfig that takes no parameters, it could use the SRV
records for “_sniproxy._tcp” queried on the current domain to discover the
proxy. Or if given just a hostname could SRV query “_sniproxy._tcp.hostname”
and fall back to A “hostname” on the default port.
This API sets the stage for a potential follow-on RFC to support other
proxy types. One way that might work would be addition of
setProxyType(String type)/setPoolProxyType(String type) methods. That RFC
might reserve the "SNI" proxy type. That RFC might specify an SPI (per
Jacob's proposal) that would let Geode users register their own proxy types
e.g. SOCKS.
Adding disconnected method to set the proxy type by string doesn’t sit well for
me. What strings are valid? When we add this method what does it mean if we
don’t set it, its always SNI?
How would you deprecate a proxy? If the proxy configuration is a class I can
discover it in my IDE and deprecate the class when not supported anymore. Even
the original Enum idea could have deprecated enum values.
I would vote for the original RFC over this given that it weakens the type
safety of the API.
To illustrate my point on the SPI see
https://github.com/apache/geode/compare/develop...pivotal-jbarrett:wip/proxy-spi
.
Its a quick hack and some things might not be perfect, like discovering the
factories, but it should get the idea across.
-Jake
Re: RFC - Client side configuration for a SNI proxy
-1
I hate to do this but I really feel like we went backwards on this change.
> On Mar 11, 2020, at 3:03 PM, Bill Burcham wrote:
> PoolFactory {
> setProxyAddress(String host, int port);
> }
>
> ClientCacheFactory {
> setPoolProxyAddress(String host, int port);
> }
It gives the user no information about what type of proxy is in use. So
documentation must specify that it is only SNI. It assumes that SNI and any
other proxy would have a host and port only.
Also consider if we used SRV records to discover the proxy, would I need to set
hostname to null or the service name, what about port since it comes from the
SRV record. Would I set port to 0?.
What about default ports for well know proxy services like SOCKS? Again, 0?
> This should address Johns desire for an API that doesn't grow with each new
> proxy type. And it should address Udo's desire for extensibility.
I really struggle to see how this satisfies future extensibility given the
parameter lock in. Providing a class allows the proxy to define all its
options. Consider SniProxyConfig that takes no parameters, it could use the SRV
records for “_sniproxy._tcp” queried on the current domain to discover the
proxy. Or if given just a hostname could SRV query “_sniproxy._tcp.hostname”
and fall back to A “hostname” on the default port.
>
> This API sets the stage for a potential follow-on RFC to support other
> proxy types. One way that might work would be addition of
> setProxyType(String type)/setPoolProxyType(String type) methods. That RFC
> might reserve the "SNI" proxy type. That RFC might specify an SPI (per
> Jacob's proposal) that would let Geode users register their own proxy types
> e.g. SOCKS.
Adding disconnected method to set the proxy type by string doesn’t sit well for
me. What strings are valid? When we add this method what does it mean if we
don’t set it, its always SNI?
How would you deprecate a proxy? If the proxy configuration is a class I can
discover it in my IDE and deprecate the class when not supported anymore. Even
the original Enum idea could have deprecated enum values.
I would vote for the original RFC over this given that it weakens the type
safety of the API.
To illustrate my point on the SPI see
https://github.com/apache/geode/compare/develop...pivotal-jbarrett:wip/proxy-spi
.
Its a quick hack and some things might not be perfect, like discovering the
factories, but it should get the idea across.
-Jake
