Mikhail, I looked into PR [1] and left a couple of minor comments on GitHub. But actually a real thing where I do not have enough expertise is a notification firing strategy. From the first glance looks fine, but there might be concerns I missed.
[1] https://github.com/apache/ignite/pull/7057 ср, 4 дек. 2019 г. в 23:02, Mikhail Petrov <pmgheap....@gmail.com>: > > Ivan, Andrey, Nikolay, > > Thanks for your answers! > > Igniters, if there are no objections, could anyone take a look at > implementation [1] of described above event, please? > > [1] https://github.com/apache/ignite/pull/7057 > > Regards, > Mikhail. > > On 04.12.2019 21:54, Ivan Pavlukhin wrote: > > Mikhail, > > > > Yes I am fine with the approach. > > > > ср, 4 дек. 2019 г. в 20:30, Mikhail Petrov <pmgheap....@gmail.com>: > >> Ivan, > >> > >> Am I right, that current approach to solving this problem looks good for > >> you? > >> > >> Regards, > >> Mikhail. > >> > >> On 03.12.2019 15:12, Ivan Pavlukhin wrote: > >>> Mikhail, > >>> > >>> Yep, I see IgniteNodeValidationResult in new event in PR [1]. > >>> > >>>> Discovery events such as (join/left/failed) are connected with a > >>>> topology version change. In my case that not happens and may be > >>>> misleading. That's why the new event type was chosen. > >>> I did not mean that one of those events should be used. I meant that > >>> it sounds natural to me to have an additional "unsuccessful node join" > >>> event (like is done in PR[1]). > >>> > >>> https://github.com/apache/ignite/pull/7057/files > >>> > >>> вт, 3 дек. 2019 г. в 14:32, Mikhail Petrov <pmgheap....@gmail.com>: > >>>> Nikolay, Ivan, > >>>> > >>>> I understood the possible problem. It seems that it can be solved using > >>>> EventStorageSpi which starts before DiscoveryManager. > >>>> > >>>> As for me, ClusterNode is enough. It contains all info about joining > >>>> node including its attributes. > >>>> > >>>> Discovery events such as (join/left/failed) are connected with a > >>>> topology version change. In my case that not happens and may be > >>>> misleading. That's why the new event type was chosen. > >>>> > >>>> The cause of the failure is also presented in the event. > >>>> > >>>> Regards, > >>>> Mikhail. > >>>> > >>>> On 03.12.2019 13:19, Николай Ижиков wrote: > >>>>> Exception(s) from component(s) that don’t want node joined. > >>>>> > >>>>>> 3 дек. 2019 г., в 12:39, Ivan Pavlukhin <vololo...@gmail.com> > >>>>>> написал(а): > >>>>>> > >>>>>> How that reason will look like? Actually, I mostly thinking about > >>>>>> general API here. What I would like to avoid is exposing something not > >>>>>> general but needed only for a particular extension (plugin). > >>>>>> > >>>>>> вт, 3 дек. 2019 г. в 12:31, Николай Ижиков <nizhi...@apache.org>: > >>>>>>> I think we also should provide the reason why join failed. > >>>>>>> > >>>>>>>> 3 дек. 2019 г., в 12:22, Ivan Pavlukhin <vololo...@gmail.com> > >>>>>>>> написал(а): > >>>>>>>> > >>>>>>>> Mikhail, > >>>>>>>> > >>>>>>>> So, I suppose there should be ordering guarantees that listener is > >>>>>>>> registered before first validation failure can occur. Hope > >>>>>>>> GridComponent#onKernalStart is the right place. Is it enough to pass > >>>>>>>> only problematic node id (or ClusterNode) with an event? Actually > >>>>>>>> such > >>>>>>>> event seems to fit naturally node join/left/failed events. > >>>>>>>> > >>>>>>>> вт, 3 дек. 2019 г. в 10:38, Mikhail Petrov <pmgheap....@gmail.com>: > >>>>>>>>> Hi Ivan. > >>>>>>>>> > >>>>>>>>> No other lifecycle events are needed in my case. > >>>>>>>>> > >>>>>>>>> We can register a listener in the security component's > >>>>>>>>> GridComponent#onKernalStart method and listen locally to every > >>>>>>>>> failed > >>>>>>>>> joining attempt. Am I missing something? > >>>>>>>>> > >>>>>>>>> Regards, > >>>>>>>>> Mikhail. > >>>>>>>>> > >>>>>>>>> On 03.12.2019 8:48, Ivan Pavlukhin wrote: > >>>>>>>>>> Mikhail, > >>>>>>>>>> > >>>>>>>>>> Do you need some ordering guarantees among node lifecycle events > >>>>>>>>>> and > >>>>>>>>>> listener notifications. For example, I can imagine that it is good > >>>>>>>>>> to > >>>>>>>>>> notify security component about every node failed validation. How > >>>>>>>>>> can > >>>>>>>>>> we achieve it with events (I assume dynamic listener registration)? > >>>>>>>>>> > >>>>>>>>>> пн, 2 дек. 2019 г. в 18:09, Mikhail Petrov <pmgheap....@gmail.com>: > >>>>>>>>>>> Hi, Andrey. > >>>>>>>>>>> > >>>>>>>>>>> It doesn't influence on authentication or authorization process. > >>>>>>>>>>> There > >>>>>>>>>>> is a security requirement that demands to notify some outer > >>>>>>>>>>> security > >>>>>>>>>>> subsystems in a specific way when joining node validation failed > >>>>>>>>>>> in any > >>>>>>>>>>> Ignite component (e.g. GridCacheProcessor) not only in > >>>>>>>>>>> IgniteSecurityProcessor. So PluginProvider#validateNewNode is not > >>>>>>>>>>> acceptable for me. > >>>>>>>>>>> > >>>>>>>>>>> Regards, > >>>>>>>>>>> Mikhail. > >>>>>>>>>>> > >>>>>>>>>>> On 02.12.2019 16:35, Andrey Gura wrote: > >>>>>>>>>>>> Mikhail, > >>>>>>>>>>>> > >>>>>>>>>>>> I don't understand how node validation on join affects security. > >>>>>>>>>>>> But > >>>>>>>>>>>> it seems that you can use > >>>>>>>>>>>> PluginProvider#validateNewNode(org.apache.ignite.cluster.ClusterNode, > >>>>>>>>>>>> java.io.Serializable) method. Does it fit for your needs? > >>>>>>>>>>>> > >>>>>>>>>>>> On Mon, Dec 2, 2019 at 12:54 PM Mikhail Petrov > >>>>>>>>>>>> <pmgheap....@gmail.com> wrote: > >>>>>>>>>>>>> Hi, Ivan. > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> Unfortunately, we came to no decision yet. As I mentioned above > >>>>>>>>>>>>> this > >>>>>>>>>>>>> event is disabled by default and no node will receive this > >>>>>>>>>>>>> event without > >>>>>>>>>>>>> an explicit subscription. In my case, that event is assumed to > >>>>>>>>>>>>> be used > >>>>>>>>>>>>> on node locally to share joining node info between security and > >>>>>>>>>>>>> discovery components. I have no idea how to solve this problem > >>>>>>>>>>>>> without > >>>>>>>>>>>>> publishing a new event too. But I'm concerned about the > >>>>>>>>>>>>> acceptance of > >>>>>>>>>>>>> that solution. Maybe it can be solved some other way? I will > >>>>>>>>>>>>> appreciate > >>>>>>>>>>>>> any suggestion or review PR [1] with event implementation. > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> [1] https://github.com/apache/ignite/pull/7057 > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> Regards, > >>>>>>>>>>>>> > >>>>>>>>>>>>> Mikhail. > >>>>>>>>>>>>> > >>>>>>>>>>>>> On 02.12.2019 10:38, Ivan Pavlukhin wrote: > >>>>>>>>>>>>>> Mikhail, Andrey, > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> Have you come to a common decision here? As for me, it sounds > >>>>>>>>>>>>>> useful > >>>>>>>>>>>>>> to expose node join failure details somehow. The thing to > >>>>>>>>>>>>>> decide on is > >>>>>>>>>>>>>> a mechanism to expose it. Unfortunately, immediately have no > >>>>>>>>>>>>>> idea > >>>>>>>>>>>>>> better than using events. > >>>>>>>>>>>>>> > >>>>>>>>>>>>>>> What is purpose of the special cluster wide event? Node is > >>>>>>>>>>>>>>> not joined > >>>>>>>>>>>>>>> to the topology. Why topology nodes should know something > >>>>>>>>>>>>>>> about this > >>>>>>>>>>>>>>> node? > >>>>>>>>>>>>>> Was this question answered? I suppose that at least > >>>>>>>>>>>>>> coordinator will > >>>>>>>>>>>>>> receive the event, will not it? > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> чт, 28 нояб. 2019 г. в 10:10, Mikhail Petrov > >>>>>>>>>>>>>> <pmgheap....@gmail.com>: > >>>>>>>>>>>>>>> Hi, Ivan. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> I'm sorry that the discussion was moved in private channel. > >>>>>>>>>>>>>>> The problem > >>>>>>>>>>>>>>> I'm trying to solve is described below in the thread. The > >>>>>>>>>>>>>>> security > >>>>>>>>>>>>>>> plugin in my case stores and analyzesinfo about a node that > >>>>>>>>>>>>>>> failed to join. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Regards, > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Mikhail. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> -------- Forwarded Message -------- > >>>>>>>>>>>>>>> Subject: Re: Joining node validation failure event. > >>>>>>>>>>>>>>> Date: Thu, 21 Nov 2019 21:43:33 +0300 > >>>>>>>>>>>>>>> From: Mikhail Petrov <pmgheap....@gmail.com> > >>>>>>>>>>>>>>> To: Andrey Gura <ag...@apache.org> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Hi, Andrey. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> In my task security plugin running on the coordinator must > >>>>>>>>>>>>>>> locally > >>>>>>>>>>>>>>> handle the situation when some node is trying to join the > >>>>>>>>>>>>>>> topology with > >>>>>>>>>>>>>>> the invalid configuration. I can't handle the response on a > >>>>>>>>>>>>>>> node that > >>>>>>>>>>>>>>> failed to connect because it's untrusted. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Actually I'm only concerned about one validation -- when all > >>>>>>>>>>>>>>> Ignite > >>>>>>>>>>>>>>> components validate new node. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> In my case plugin must be able to obtain general and security > >>>>>>>>>>>>>>> subject > >>>>>>>>>>>>>>> information from joining TcpDiscoveryNode attributes. But I > >>>>>>>>>>>>>>> have no idea > >>>>>>>>>>>>>>> how to share information between the security and discovery > >>>>>>>>>>>>>>> components > >>>>>>>>>>>>>>> without recording event and listening it locally. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> This event is assumed to be disable by default and in my case > >>>>>>>>>>>>>>> used only > >>>>>>>>>>>>>>> on local node so it's not look like "cluster wide event". > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Also I propose to record this event in dedicated utilityPool > >>>>>>>>>>>>>>> so it can't > >>>>>>>>>>>>>>> stuck discovery thread. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> I will appreciate any thoughts on this problem. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Regards, > >>>>>>>>>>>>>>> Mikhail. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> On 21.11.2019 19:40, Andrey Gura wrote: > >>>>>>>>>>>>>>>> Mikhail, > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> It is still not enough details to me. What is expected > >>>>>>>>>>>>>>>> behavior if the > >>>>>>>>>>>>>>>> plugin? > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> There are a different validations during node join. Many of > >>>>>>>>>>>>>>>> them > >>>>>>>>>>>>>>>> placed in RingMessageWorker#processJoinRequestMessage > >>>>>>>>>>>>>>>> method. If > >>>>>>>>>>>>>>>> validation will fail then corresponding message will be sent > >>>>>>>>>>>>>>>> to the > >>>>>>>>>>>>>>>> joining node (including TcpDiscoveryAuthFailedMessage) and > >>>>>>>>>>>>>>>> node will > >>>>>>>>>>>>>>>> not joined to topology. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> What is purpose of the special cluster wide event? Node is > >>>>>>>>>>>>>>>> not joined > >>>>>>>>>>>>>>>> to the topology. Why topology nodes should know something > >>>>>>>>>>>>>>>> about this > >>>>>>>>>>>>>>>> node? > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> On Thu, Nov 21, 2019 at 9:54 AM Mikhail Petrov > >>>>>>>>>>>>>>>> <pmgheap....@gmail.com> > >>>>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>>>> Hi, Andrey. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> I take part in the development of a custom security plugin > >>>>>>>>>>>>>>>>> for Apache > >>>>>>>>>>>>>>>>> Ignite. There is an information security requirement for > >>>>>>>>>>>>>>>>> which node > >>>>>>>>>>>>>>>>> joining failures due to invalid configuration must be > >>>>>>>>>>>>>>>>> handled by the > >>>>>>>>>>>>>>>>> plugin. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> This is where my case comes from. Are there any objections > >>>>>>>>>>>>>>>>> to the > >>>>>>>>>>>>>>>>> proposed approach? > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Regards, > >>>>>>>>>>>>>>>>> Mikhail. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> On 20.11.2019 19:38, Andrey Gura wrote: > >>>>>>>>>>>>>>>>>> Hi, Mikhail! > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> Could you please describe the case for this new event? > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> On Wed, Nov 20, 2019 at 12:45 PM Mikhail Petrov > >>>>>>>>>>>>>>>>>> <pmgheap....@gmail.com> wrote: > >>>>>>>>>>>>>>>>>>> Hello, Igniters. > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> There is a case which requires to handle joining node > >>>>>>>>>>>>>>>>>>> validation > >>>>>>>>>>>>>>>>>>> failure > >>>>>>>>>>>>>>>>>>> in Ignite components and obtain information of the node > >>>>>>>>>>>>>>>>>>> that tried to > >>>>>>>>>>>>>>>>>>> join and the reason for the failure. Now, as I see, there > >>>>>>>>>>>>>>>>>>> is no way to > >>>>>>>>>>>>>>>>>>> do it. I propose to implement a new event -- > >>>>>>>>>>>>>>>>>>> NodeValidationFailedEvent > >>>>>>>>>>>>>>>>>>> -- and record it in case the validation fails. I have > >>>>>>>>>>>>>>>>>>> created Tiket [1] > >>>>>>>>>>>>>>>>>>> and PR [2], which shows an example of implementation. > >>>>>>>>>>>>>>>>>>> Could anyone take > >>>>>>>>>>>>>>>>>>> a look at it, please? > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-12380 > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> [2] https://github.com/apache/ignite/pull/7057 > >>>>>>>>>>>>>>>>>>> > >>>>>>>> -- > >>>>>>>> Best regards, > >>>>>>>> Ivan Pavlukhin > >>>>>> -- > >>>>>> Best regards, > >>>>>> Ivan Pavlukhin > >>> > > > > -- Best regards, Ivan Pavlukhin
