Hi Chris! Thanks for your feedback! I'll number my responses to your questions / thoughts.
1. Apologies on that lack of clarity! I settled on "Detailed exception information has been suppressed. Please see logs." (https://github.com/apache/kafka/pull/8355/files#diff-64c265986e7bbe40cdd79f831e961907R34). Should I update the KIP to reflect what I've already thought about? It's my first one, not sure what the process should be for editing. 2. I was unaware of the REST extensions! I'll see if I can implement the same behavior as a REST extension. I agree that the KIP still has merit, regardless of the feasibility of the extension, but in regards to the 5th thought, this might make that decision easier. 3. I agree with your suggestion here. Absolutely ready to take the community feedback on what makes sense here. 4. I should note that while I emphasized uncaught exceptions, I mean all exceptions handled by the ExceptionMapper, including ConnectRestExceptions. An example of this is here: https://github.com/apache/kafka/pull/8355/files#diff-64c265986e7bbe40cdd79f831e961907R46 5. I didn't know how specific I should get if I had already taken a stab at implementing! I'm happy to edit this in whatever way we want to go about it. Let me know if anyone has any other questions or feedback! Thanks! Connor On 4/2/20, 3:58 PM, "Christopher Egerton" <chr...@confluent.io> wrote: Hi Connor, Great stuff! I generally like being able to see the stack trace of an exception directly via the REST API but can definitely understand the security concerns here. I've got a few questions/remarks about the KIP and would be interested in your thoughts: 1. The KIP mentions a SUPRESSED_EXCEPTION_MESSAGE, but doesn't actually outline what this message would actually be. It'd be great to see the actual message in the KIP since people may have thoughts on what it should be and want to comment on it as part of this discussion. 2. In the "Rejected Alternatives" section, an Nginx proxy is mentioned as one possible way to filter out stack traces from the REST API. It seems like a Connect REST extension ( https://kafka.apache.org/24/javadoc/index.html?org/apache/kafka/connect/rest/ConnectRestExtension.html) would be a better alternative than an Nginx proxy; had you considered utilizing one? I still think this KIP is worthwhile and a REST extension shouldn't be necessary in order to lock down the REST API this way, but it might be worth calling out as an alternative and perhaps even a workaround in cases where users are stuck on a given version of Connect and can't upgrade to 2.6 (or whichever version this KIP lands on) any time soon. 3. The "error.rest.response.message.detail.enabled" property is a bit of a mouthful; it'd be great if we could come up with something more succinct. What do you think about something like "rest.response.stack.traces"? 4. The KIP is targeted at stack traces for uncaught exceptions, but it's also possible that stack traces get exposed in the REST API when querying the status of a connector or one of its tasks. Was this intentional? If so, it'd be great to call out why that kind of filtering is not required in the "Rejected Alternatives" section, and if not, it's probably not too late to consider modifying the KIP to cover those cases as well. 5. The KIP mentions creating a new, separate exception mapper class. This seems like more of an implementation detail and something that can be decided on during code review; unless it's critical to the functionality that the KIP aims to accomplish, I'd suggest leaving that part out since it shouldn't affect the impact on users of the Connect framework. Thanks for the KIP, looking forward to seeing this happen! Cheers, Chris On Thu, Apr 2, 2020 at 11:01 AM Connor Penhale <cpenh...@perforce.com> wrote: > Hello All! > > I’ve created the following KIP: > https://cwiki.apache.org/confluence/display/KAFKA/KIP-587:+Suppress+detailed+responses+for+handled+exceptions+in+security-sensitive+environments > > The PR that originated this discussion, is here: > https://github.com/apache/kafka/pull/8355 It is based on 2.0, but I > would be working on Kafka Connect in 2.6 to get this behavior changed to > the community’s preference. > > Looking forward to working with everyone! > > Thanks! > Connor > --- > Connor Penhale | Enterprise Architect, OpenLogic (https://openlogic.com/) > Perforce (https://www.perforce.com/) > Support: +1 866.399.6736 > > > > > This e-mail may contain information that is privileged or confidential. If > you are not the intended recipient, please delete the e-mail and any > attachments and notify us immediately. > > CAUTION: This email originated from outside of the organization. Do not click on links or open attachments unless you recognize the sender and know the content is safe. This e-mail may contain information that is privileged or confidential. If you are not the intended recipient, please delete the e-mail and any attachments and notify us immediately.
