[image: stefan.miklosovic]Stefan Miklosovic <https://issues.apache.org/jira/secure/ViewProfile.jspa?name=stefan.miklosovic> added a comment - 01/Jul/21 19:20
I ve taken a look too. Added some comments to PR. It would be awesome if we see some 3rd party implementation of this in action so we know it indeed works as intended. It is strange to just code up an interface by default logic for which it works but there isnt any (public) example how to do yet another impl. On Fri, Jul 9, 2021 at 2:57 PM Maulin Vasavada <maulin.vasav...@gmail.com> wrote: > Sumanth Pasupuleti > <https://issues.apache.org/jira/secure/ViewProfile.jspa?name=sumanth.pasupuleti> > added > a comment - 07/Jun/21 15:13 > > > Maulin Vasavada > <https://issues.apache.org/jira/secure/ViewProfile.jspa?name=maulin.vasavada> > left > a couple of review comments, but lgtm overall. > > One of the things I was hoping we can also achieve is to be able to > provide mechanics to transparently transition from using one SSLFactory > implementation to another, and using those mechanics, one could do the > following on their cluster for moving from say Implementation1 to > Implementation2 > Stage #1: Current state of being only Implementation1 aware. Use keystore > and trustmanager of implementation1 > Stage #2: Start trusting both implementation1 and implementation2. Use > keystore of implementation1, but use trustmanager of both implementation1 > and implementation2 (using MultiTrustManagerFactory) (and perform a rolling > restart of the cluster) > Stage #3: Start using implementation2 for keystore, and perform a rolling > restart of the cluster > Stage #4: At this point, all nodes of the cluster are using > implementation2 for keystore, but trust both implementation1 and > implementation2, and we can now remove implementation1 from trustmanagers, > and do a rolling restart. > > Since this ticket is about making SSLContext pluggable, I believe this is > out of scope; cut a separate ticket CASSANDRA-16719 > <https://issues.apache.org/jira/browse/CASSANDRA-16719> to track that > work (I did an internal 3.0 patch for this transition work, and I can try > porting it to 4.x once this ticket is committed) > > On Fri, Jul 9, 2021 at 2:56 PM Maulin Vasavada <maulin.vasav...@gmail.com> > wrote: > >> Hi all >> >> I wanted to consolidate a couple of comments that started in JIRA/Wiki >> here to keep it in one place. I'll post different posts as replies for each >> comment. >> >> Thanks >> Maulin >> >> On Tue, Jun 29, 2021 at 1:07 PM Maulin Vasavada < >> maulin.vasav...@gmail.com> wrote: >> >>> ^^^ bumping up ^^^ this thread since people might have more time >>> reviewing post 4.0 work. Specifically for this >>> <https://cwiki.apache.org/confluence/display/CASSANDRA/CEP-9%3A+Make+SSLContext+creation+pluggable#CEP9:MakeSSLContextcreationpluggable-ImportantnoteaboutcommonSSLconfigurations> >>> section in the CEP, I have coded for one option (here >>> <https://github.com/maulin-vasavada/cassandra/commit/256ad30ecedbc50d66d8a039f8ca9e47074737ce>) >>> and now will do for another option very soon. >>> >>> On Wed, Jun 2, 2021 at 5:11 PM Maulin Vasavada < >>> maulin.vasav...@gmail.com> wrote: >>> >>>> Thank you Dinesh and everybody. Will keep calm and wait for the >>>> feedback. Meanwhile I am experimenting with various implementation options >>>> for what I put as "will seek community's input >>>> <https://cwiki.apache.org/confluence/display/CASSANDRA/CEP-9%3A+Make+SSLContext+creation+pluggable#CEP9:MakeSSLContextcreationpluggable-ImportantnoteaboutcommonSSLconfigurations>" >>>> on the CEP document and learning little bit more about the CircleCI. >>>> >>>> On Wed, Jun 2, 2021 at 4:08 PM Dinesh Joshi <djos...@icloud.com.invalid> >>>> wrote: >>>> >>>>> Hi Maulin, >>>>> >>>>> Thank you for the CEP & Patch. I’ve been following along albeit >>>>> silently. Will take a look. It’s just that we’re currently busy so bear >>>>> with us. >>>>> >>>>> Thanks, >>>>> >>>>> Dinesh >>>>> >>>>> > On Jun 2, 2021, at 3:28 PM, Maulin Vasavada < >>>>> maulin.vasav...@gmail.com> wrote: >>>>> > >>>>> > Hi all >>>>> > >>>>> > ^^^ bump ^^^ I've raised the PR and am waiting for the review. Once >>>>> I see >>>>> > that the suggested changes are directionally right I'll start a VOTE >>>>> thread >>>>> > on the CEP (unless I am recommended to follow another process). >>>>> > >>>>> > Thanks >>>>> > Maulin >>>>> > >>>>> >> On Thu, May 27, 2021 at 1:29 PM Maulin Vasavada < >>>>> maulin.vasav...@gmail.com> >>>>> >> wrote: >>>>> >> >>>>> >> HI all >>>>> >> >>>>> >> I've raised the PR with the changes. Specifically I would >>>>> appreciate the >>>>> >> community's input on this section of the CEP >>>>> >> < >>>>> https://cwiki.apache.org/confluence/display/CASSANDRA/CEP-9%3A+Make+SSLContext+creation+pluggable#CEP9:MakeSSLContextcreationpluggable-ImportantnoteaboutcommonSSLconfigurations >>>>> > >>>>> >> . >>>>> >> >>>>> >> Once we get some consensus on the PR (except minor code improvement >>>>> >> suggestions) I'll start a VOTE thread for the CEP. >>>>> >> >>>>> >> I thank all the reviewers of the CEP and the PR in advance and am >>>>> >> completely excited to contribute to Apache Cassandra. >>>>> >> >>>>> >> Thanks >>>>> >> Maulin >>>>> >> >>>>> >> On Thu, May 27, 2021 at 11:04 AM Maulin Vasavada < >>>>> >> maulin.vasav...@gmail.com> wrote: >>>>> >> >>>>> >>> Sounds good Brandon. I'll raise the PR in a couple of hours from >>>>> now. >>>>> >>> Thanks. >>>>> >>> >>>>> >>> On Thu, May 27, 2021 at 10:14 AM Brandon Williams < >>>>> dri...@gmail.com> >>>>> >>> wrote: >>>>> >>> >>>>> >>>> You can raise a PR in any state, but review will be a different >>>>> >>>> matter. I would go ahead and raise it and the testing can be >>>>> sorted >>>>> >>>> out from there. >>>>> >>>> >>>>> >>>> On Thu, May 27, 2021 at 12:12 PM Maulin Vasavada >>>>> >>>> <maulin.vasav...@gmail.com> wrote: >>>>> >>>>> >>>>> >>>>> Hi all >>>>> >>>>> >>>>> >>>>> I think I am close to raising a PR now but my CircleCI job >>>>> >>>>> < >>>>> https://app.circleci.com/pipelines/github/maulin-vasavada/cassandra> >>>>> >>>>> doesn't make progress beyond key tasks success like unit tests, >>>>> dtests, >>>>> >>>>> cqlshlibtests. Any recommendation on if we need to see the whole >>>>> >>>> CircleCI >>>>> >>>>> job green before raising the PR? >>>>> >>>>> >>>>> >>>>> Thanks >>>>> >>>>> Maulin >>>>> >>>>> >>>>> >>>>> On Fri, May 21, 2021 at 8:54 PM Maulin Vasavada < >>>>> >>>> maulin.vasav...@gmail.com> >>>>> >>>>> wrote: >>>>> >>>>> >>>>> >>>>>> I am almost done with all changes except the code snippet in the >>>>> >>>>>> EncryptioOptions.java which determines 'enabled' and 'optional' >>>>> >>>> encryption >>>>> >>>>>> flags. Will raise the PR soon once I see my CircleCI getting >>>>> green. >>>>> >>>>>> >>>>> >>>>>> On Fri, May 21, 2021 at 9:24 AM Maulin Vasavada < >>>>> >>>> maulin.vasav...@gmail.com> >>>>> >>>>>> wrote: >>>>> >>>>>> >>>>> >>>>>>> FYI - I am working on PR. I made some changes and trying to run >>>>> >>>> tests. >>>>> >>>>>>> >>>>> >>>>>>> On Tue, May 18, 2021 at 10:14 PM Maulin Vasavada < >>>>> >>>>>>> maulin.vasav...@gmail.com> wrote: >>>>> >>>>>>> >>>>> >>>>>>>> Thanks Nate for reviewing the CEP. Yes for change #3 in the >>>>> CEP, I >>>>> >>>> mean >>>>> >>>>>>>> to have only single Default Impl and that would be a final >>>>> class, >>>>> >>>> not >>>>> >>>>>>>> overridable. It will be basically an internal implementation. >>>>> I've >>>>> >>>> updated >>>>> >>>>>>>> the CEP to reflect this. >>>>> >>>>>>>> >>>>> >>>>>>>> On Tue, May 18, 2021 at 7:21 PM Nate McCall < >>>>> zznat...@gmail.com> >>>>> >>>> wrote: >>>>> >>>>>>>> >>>>> >>>>>>>>> Hi Maulin, >>>>> >>>>>>>>> Thanks for putting this together! >>>>> >>>>>>>>> >>>>> >>>>>>>>> Took a quick glance, and I can't think of a compelling >>>>> reason on >>>>> >>>> why >>>>> >>>>>>>>> SSLContext should be final and your point about >>>>> >>>> organization/compliance >>>>> >>>>>>>>> issues requiring different implementations is a good one. >>>>> >>>>>>>>> >>>>> >>>>>>>>> Per #3 on your proposed changes, I'm keen to only support a >>>>> single >>>>> >>>>>>>>> default >>>>> >>>>>>>>> impl in-tree. I don't think we should be in the business of >>>>> >>>> picking >>>>> >>>>>>>>> implementation to support. It looks like this is your >>>>> intention >>>>> >>>> as well? >>>>> >>>>>>>>> >>>>> >>>>>>>>> Thanks again, >>>>> >>>>>>>>> -Nate >>>>> >>>>>>>>> >>>>> >>>>>>>>> On Wed, May 19, 2021 at 12:05 PM Maulin Vasavada < >>>>> >>>>>>>>> maulin.vasav...@gmail.com> >>>>> >>>>>>>>> wrote: >>>>> >>>>>>>>> >>>>> >>>>>>>>>> Hi all >>>>> >>>>>>>>>> >>>>> >>>>>>>>>> Starting a discussion thread for the CIP-9 - >>>>> >>>>>>>>>> >>>>> >>>>>>>>>> >>>>> >>>>>>>>> >>>>> >>>> >>>>> https://cwiki.apache.org/confluence/display/CASSANDRA/CEP-9%3A+Make+SSLContext+creation+pluggable >>>>> >>>>>>>>>> >>>>> >>>>>>>>>> >>>>> >>>>>>>>>> However, while writing the CIP two areas that came up in my >>>>> mind >>>>> >>>>>>>>> where I >>>>> >>>>>>>>>> need to seek guidance apart from the other discussion that >>>>> we >>>>> >>>> would >>>>> >>>>>>>>> have >>>>> >>>>>>>>>> here, >>>>> >>>>>>>>>> >>>>> >>>>>>>>>> 1. Whether to consider >>>>> >>>> SSLFactory#tlsInstanceProtocolSubstitution() >>>>> >>>>>>>>>> < >>>>> >>>>>>>>>> >>>>> >>>>>>>>> >>>>> >>>> >>>>> https://github.com/apache/cassandra/blob/cassandra-4.0/src/java/org/apache/cassandra/security/SSLFactory.java#L169 >>>>> >>>>>>>>>>> >>>>> >>>>>>>>>> for pluggability (noted this on the CIP as well) >>>>> >>>>>>>>>> >>>>> >>>>>>>>>> 2. For Test Plan, apart from Integration Test and local >>>>> system >>>>> >>>> test >>>>> >>>>>>>>> what >>>>> >>>>>>>>>> would be recommended? >>>>> >>>>>>>>>> >>>>> >>>>>>>>>> Thanks >>>>> >>>>>>>>>> Maulin >>>>> >>>>>>>>>> >>>>> >>>>>>>>> >>>>> >>>>>>>> >>>>> >>>> >>>>> >>>> >>>>> --------------------------------------------------------------------- >>>>> >>>> To unsubscribe, e-mail: dev-unsubscr...@cassandra.apache.org >>>>> >>>> For additional commands, e-mail: dev-h...@cassandra.apache.org >>>>> >>>> >>>>> >>>> >>>>> >>>>> --------------------------------------------------------------------- >>>>> To unsubscribe, e-mail: dev-unsubscr...@cassandra.apache.org >>>>> For additional commands, e-mail: dev-h...@cassandra.apache.org >>>>> >>>>>
