You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Zhiguo Huang <je...@confluent.io> on 2020/03/13 00:05:11 UTC
[DISCUSS] KIP-577: Allow HTTP Response Headers Configured for Kafka Connect
Re: [DISCUSS] KIP-577: Allow HTTP Response Headers Configured for Kafka Connect
Posted by Jeff Huang <je...@confluent.io>.
1. Updated KIP adding prefix response.http.headers.{name}.
2. Updated KIP using a table to describe type and validate values.
3. Added meaningful examples for Connect scenarios.
4. We use comma to separate different HTTP headers. But we do allow the header value contain comma, just need use double quotes for that HTTP header.
On 2020/03/18 18:06:30, Randall Hauch <rh...@gmail.com> wrote:
> Thanks for the proposal, Jeff. I can see how this proposal will add value.
>
> I have a few comments, most of which are asking for more detail in the KIP.
>
> 1. I do think the KIP needs to be much more explicit about what the new
> configuration properties will be and to use them consistently. For example,
> one of the examples shows using the "header.config" property, but the first
> paragraph in the proposal talks about using the "response.http.headers."
> prefix on all such properties. If that's the case, then all examples should
> include this prefix. We know from experience that users will copy examples
> from KIPs and paste directly into their worker configs.
> 2. The KIP should be very clear about the types of each new property and
> the validation rules that will be implemented. The more validation we
> perform, the less opportunity for users to mis-configure these properties.
> The test plan should also list all of the cases that the tests should
> cover, including positive cases to verify they work correctly and negative
> cases to verify that invalid configurations are caught properly.
> 3. I infer that the purpose of the include and exclude paths is to
> control which headers are applied to which of the Connect REST API's
> resources. But it'd be good to be more explicit and to have the examples be
> more meaningful. Also, the motivation section should clearly spell out why
> different headers on different resources, and should maybe provide a
> concrete example use case.
> 4. The "header.config" example has a comma outside of the quotes, and
> this seems to not align with the statement before about having to use
> commas. How will the example even be parsed? Seems like we need more
> explanation here, and possibly more examples.
> 5. Although the proposal seems to follow the pattern used for SMTs, this
> still is a complicated pattern. Is there any way to simplify the proposal?
>
> Best regards,
>
> Randall
>
> On Fri, Mar 13, 2020 at 4:04 PM Jeff Huang <je...@confluent.io> wrote:
>
> > Hi Aneel,
> >
> > It is great idea. I will update KIP based on your suggestion.
> >
> > Jeff.
> >
> > On 2020/03/13 18:58:23, Aneel Nazareth <an...@confluent.io> wrote:
> > > If we're including and excluding paths, it seems like it might make
> > > sense to allow for the configuration of multiple filters.
> > >
> > > We could do this with a pattern similar to how Kafka listeners are
> > > configured. Something like:
> > >
> > > response.http.header.filters = myfilter1,myfilter2
> > > response.http.header.myfilter1.included.paths = ...
> > > response.http.header.myfilter1.included.mime.types = ...
> > > response.http.header.myfilter1.config = set X-Frame-Options: DENY,"add
> > > Cache-Control: no-cache, no-store, must-revalidate", ...
> > >
> > > response.http.header.myfilter2.included.paths = ...
> > > response.http.header.myfilter2.included.mime.types = ...
> > > response.http.header.myfilter2.config = setDate Expires: 31540000000 ...
> > >
> > > But before we go down that road: are people going to want to be able
> > > to set multiple different header filters? Or is one header filter for
> > > all of the responses good enough?
> > >
> > > On Fri, Mar 13, 2020 at 10:56 AM Jeff Huang <je...@confluent.io>
> > wrote:
> > > >
> > > > Hi Aneel,
> > > >
> > > > That is really great point. I will update KIP. We need add following
> > properties combining with header configs:
> > > > includedPaths - CSV of path specs to include
> > > > excludedPaths - CSV of path specs to exclude
> > > > includedMimeTypes - CSV of mime types to include
> > > > excludedMimeTypes - CSV of mime types to exclude
> > > > includedHttpMethods - CSV of http methods to include
> > > > excludedHttpMethods - CSV of http methods to exclude
> > > >
> > > > Jeff.
> > > >
> > > > On 2020/03/13 14:28:11, Aneel Nazareth <an...@confluent.io> wrote:
> > > > > Hi Jeff,
> > > > >
> > > > > Thanks for the KIP.
> > > > >
> > > > > Will users always want to set identical headers on all responses?
> > Does
> > > > > it make sense to also allow configuration of the HeaderFilter init
> > > > > parameters like "includedPaths", "excludedHttpMethods", etc.? Does it
> > > > > make sense to allow multiple configurations (so that eg. different
> > > > > paths have different headers?)
> > > > >
> > > > > Cheers,
> > > > > Aneel
> > > > >
> > > > > On Thu, Mar 12, 2020 at 7:05 PM Zhiguo Huang <
> > jeff.huang@confluent.io> wrote:
> > > > > >
> > > > > >
> > > > >
> > >
> >
>
Re: [DISCUSS] KIP-577: Allow HTTP Response Headers Configured for
Kafka Connect
Posted by Randall Hauch <rh...@gmail.com>.
Thanks for the proposal, Jeff. I can see how this proposal will add value.
I have a few comments, most of which are asking for more detail in the KIP.
1. I do think the KIP needs to be much more explicit about what the new
configuration properties will be and to use them consistently. For example,
one of the examples shows using the "header.config" property, but the first
paragraph in the proposal talks about using the "response.http.headers."
prefix on all such properties. If that's the case, then all examples should
include this prefix. We know from experience that users will copy examples
from KIPs and paste directly into their worker configs.
2. The KIP should be very clear about the types of each new property and
the validation rules that will be implemented. The more validation we
perform, the less opportunity for users to mis-configure these properties.
The test plan should also list all of the cases that the tests should
cover, including positive cases to verify they work correctly and negative
cases to verify that invalid configurations are caught properly.
3. I infer that the purpose of the include and exclude paths is to
control which headers are applied to which of the Connect REST API's
resources. But it'd be good to be more explicit and to have the examples be
more meaningful. Also, the motivation section should clearly spell out why
different headers on different resources, and should maybe provide a
concrete example use case.
4. The "header.config" example has a comma outside of the quotes, and
this seems to not align with the statement before about having to use
commas. How will the example even be parsed? Seems like we need more
explanation here, and possibly more examples.
5. Although the proposal seems to follow the pattern used for SMTs, this
still is a complicated pattern. Is there any way to simplify the proposal?
Best regards,
Randall
On Fri, Mar 13, 2020 at 4:04 PM Jeff Huang <je...@confluent.io> wrote:
> Hi Aneel,
>
> It is great idea. I will update KIP based on your suggestion.
>
> Jeff.
>
> On 2020/03/13 18:58:23, Aneel Nazareth <an...@confluent.io> wrote:
> > If we're including and excluding paths, it seems like it might make
> > sense to allow for the configuration of multiple filters.
> >
> > We could do this with a pattern similar to how Kafka listeners are
> > configured. Something like:
> >
> > response.http.header.filters = myfilter1,myfilter2
> > response.http.header.myfilter1.included.paths = ...
> > response.http.header.myfilter1.included.mime.types = ...
> > response.http.header.myfilter1.config = set X-Frame-Options: DENY,"add
> > Cache-Control: no-cache, no-store, must-revalidate", ...
> >
> > response.http.header.myfilter2.included.paths = ...
> > response.http.header.myfilter2.included.mime.types = ...
> > response.http.header.myfilter2.config = setDate Expires: 31540000000 ...
> >
> > But before we go down that road: are people going to want to be able
> > to set multiple different header filters? Or is one header filter for
> > all of the responses good enough?
> >
> > On Fri, Mar 13, 2020 at 10:56 AM Jeff Huang <je...@confluent.io>
> wrote:
> > >
> > > Hi Aneel,
> > >
> > > That is really great point. I will update KIP. We need add following
> properties combining with header configs:
> > > includedPaths - CSV of path specs to include
> > > excludedPaths - CSV of path specs to exclude
> > > includedMimeTypes - CSV of mime types to include
> > > excludedMimeTypes - CSV of mime types to exclude
> > > includedHttpMethods - CSV of http methods to include
> > > excludedHttpMethods - CSV of http methods to exclude
> > >
> > > Jeff.
> > >
> > > On 2020/03/13 14:28:11, Aneel Nazareth <an...@confluent.io> wrote:
> > > > Hi Jeff,
> > > >
> > > > Thanks for the KIP.
> > > >
> > > > Will users always want to set identical headers on all responses?
> Does
> > > > it make sense to also allow configuration of the HeaderFilter init
> > > > parameters like "includedPaths", "excludedHttpMethods", etc.? Does it
> > > > make sense to allow multiple configurations (so that eg. different
> > > > paths have different headers?)
> > > >
> > > > Cheers,
> > > > Aneel
> > > >
> > > > On Thu, Mar 12, 2020 at 7:05 PM Zhiguo Huang <
> jeff.huang@confluent.io> wrote:
> > > > >
> > > > >
> > > >
> >
>
Re: [DISCUSS] KIP-577: Allow HTTP Response Headers Configured for Kafka Connect
Posted by Jeff Huang <je...@confluent.io>.
Hi Aneel,
It is great idea. I will update KIP based on your suggestion.
Jeff.
On 2020/03/13 18:58:23, Aneel Nazareth <an...@confluent.io> wrote:
> If we're including and excluding paths, it seems like it might make
> sense to allow for the configuration of multiple filters.
>
> We could do this with a pattern similar to how Kafka listeners are
> configured. Something like:
>
> response.http.header.filters = myfilter1,myfilter2
> response.http.header.myfilter1.included.paths = ...
> response.http.header.myfilter1.included.mime.types = ...
> response.http.header.myfilter1.config = set X-Frame-Options: DENY,"add
> Cache-Control: no-cache, no-store, must-revalidate", ...
>
> response.http.header.myfilter2.included.paths = ...
> response.http.header.myfilter2.included.mime.types = ...
> response.http.header.myfilter2.config = setDate Expires: 31540000000 ...
>
> But before we go down that road: are people going to want to be able
> to set multiple different header filters? Or is one header filter for
> all of the responses good enough?
>
> On Fri, Mar 13, 2020 at 10:56 AM Jeff Huang <je...@confluent.io> wrote:
> >
> > Hi Aneel,
> >
> > That is really great point. I will update KIP. We need add following properties combining with header configs:
> > includedPaths - CSV of path specs to include
> > excludedPaths - CSV of path specs to exclude
> > includedMimeTypes - CSV of mime types to include
> > excludedMimeTypes - CSV of mime types to exclude
> > includedHttpMethods - CSV of http methods to include
> > excludedHttpMethods - CSV of http methods to exclude
> >
> > Jeff.
> >
> > On 2020/03/13 14:28:11, Aneel Nazareth <an...@confluent.io> wrote:
> > > Hi Jeff,
> > >
> > > Thanks for the KIP.
> > >
> > > Will users always want to set identical headers on all responses? Does
> > > it make sense to also allow configuration of the HeaderFilter init
> > > parameters like "includedPaths", "excludedHttpMethods", etc.? Does it
> > > make sense to allow multiple configurations (so that eg. different
> > > paths have different headers?)
> > >
> > > Cheers,
> > > Aneel
> > >
> > > On Thu, Mar 12, 2020 at 7:05 PM Zhiguo Huang <je...@confluent.io> wrote:
> > > >
> > > >
> > >
>
Re: [DISCUSS] KIP-577: Allow HTTP Response Headers Configured for
Kafka Connect
Posted by Aneel Nazareth <an...@confluent.io>.
If we're including and excluding paths, it seems like it might make
sense to allow for the configuration of multiple filters.
We could do this with a pattern similar to how Kafka listeners are
configured. Something like:
response.http.header.filters = myfilter1,myfilter2
response.http.header.myfilter1.included.paths = ...
response.http.header.myfilter1.included.mime.types = ...
response.http.header.myfilter1.config = set X-Frame-Options: DENY,"add
Cache-Control: no-cache, no-store, must-revalidate", ...
response.http.header.myfilter2.included.paths = ...
response.http.header.myfilter2.included.mime.types = ...
response.http.header.myfilter2.config = setDate Expires: 31540000000 ...
But before we go down that road: are people going to want to be able
to set multiple different header filters? Or is one header filter for
all of the responses good enough?
On Fri, Mar 13, 2020 at 10:56 AM Jeff Huang <je...@confluent.io> wrote:
>
> Hi Aneel,
>
> That is really great point. I will update KIP. We need add following properties combining with header configs:
> includedPaths - CSV of path specs to include
> excludedPaths - CSV of path specs to exclude
> includedMimeTypes - CSV of mime types to include
> excludedMimeTypes - CSV of mime types to exclude
> includedHttpMethods - CSV of http methods to include
> excludedHttpMethods - CSV of http methods to exclude
>
> Jeff.
>
> On 2020/03/13 14:28:11, Aneel Nazareth <an...@confluent.io> wrote:
> > Hi Jeff,
> >
> > Thanks for the KIP.
> >
> > Will users always want to set identical headers on all responses? Does
> > it make sense to also allow configuration of the HeaderFilter init
> > parameters like "includedPaths", "excludedHttpMethods", etc.? Does it
> > make sense to allow multiple configurations (so that eg. different
> > paths have different headers?)
> >
> > Cheers,
> > Aneel
> >
> > On Thu, Mar 12, 2020 at 7:05 PM Zhiguo Huang <je...@confluent.io> wrote:
> > >
> > >
> >
Re: [DISCUSS] KIP-577: Allow HTTP Response Headers Configured for Kafka Connect
Posted by Jeff Huang <je...@confluent.io>.
Hi Aneel,
That is really great point. I will update KIP. We need add following properties combining with header configs:
includedPaths - CSV of path specs to include
excludedPaths - CSV of path specs to exclude
includedMimeTypes - CSV of mime types to include
excludedMimeTypes - CSV of mime types to exclude
includedHttpMethods - CSV of http methods to include
excludedHttpMethods - CSV of http methods to exclude
Jeff.
On 2020/03/13 14:28:11, Aneel Nazareth <an...@confluent.io> wrote:
> Hi Jeff,
>
> Thanks for the KIP.
>
> Will users always want to set identical headers on all responses? Does
> it make sense to also allow configuration of the HeaderFilter init
> parameters like "includedPaths", "excludedHttpMethods", etc.? Does it
> make sense to allow multiple configurations (so that eg. different
> paths have different headers?)
>
> Cheers,
> Aneel
>
> On Thu, Mar 12, 2020 at 7:05 PM Zhiguo Huang <je...@confluent.io> wrote:
> >
> >
>
Re: [DISCUSS] KIP-577: Allow HTTP Response Headers Configured for
Kafka Connect
Posted by Aneel Nazareth <an...@confluent.io>.
Hi Jeff,
Thanks for the KIP.
Will users always want to set identical headers on all responses? Does
it make sense to also allow configuration of the HeaderFilter init
parameters like "includedPaths", "excludedHttpMethods", etc.? Does it
make sense to allow multiple configurations (so that eg. different
paths have different headers?)
Cheers,
Aneel
On Thu, Mar 12, 2020 at 7:05 PM Zhiguo Huang <je...@confluent.io> wrote:
>
>