I implemented it without changing the default of the property. Now it will be allowed to execute system tasks even if ThinClientConfiguration#setMaxActiveComputeTasksPerConnection is 0. This makes it possible to run management tasks even when the user has explicitly disabled the execution of user tasks (thanks to Nikolay Izhikov for the argument in the review).
I have updated the design page [1]. If there are no objections, I will merge it soon [2]. [1] https://cwiki.apache.org/confluence/display/IGNITE/%5BPhase+3%5D+Use+IgniteClient+instead+of+GridClient+in+the+control.sh+utility [2] https://github.com/apache/ignite/pull/11648 On Wed, Nov 13, 2024 at 5:05 PM Pavel Tupitsyn <ptupit...@apache.org> wrote: > > Ok, you have convinced me, thanks. > Let's make sure to reflect the default value change in the release notes. > > On Tue, Nov 12, 2024 at 1:26 PM Nikita Amelchev <namelc...@apache.org> > wrote: > > > > Changing the default behavior to a less secure one > > > > My point is that both options represent security issues. > > > > If we want to migrate to a thin client, we need to allow management > > tasks to be executed anyway. Among them are those that can read and > > delete user data (cache scan/clear/destroy commands), deactivate a > > cluster, etc. > > Which without an explicitly configured security plugin will be a > > security issue. Isn't that so? > > > > > Mixing management tasks and user-defined tasks > > > > There is authorization logic in GridTaskProcessor to distinguish them. > > > > > Are we aware of any extensions that provide their own management > > commands? How does it work with GridClient? Invoke by class name? > > > > See CommandsProvider. Yes, they are invoked by the class name. > > > > On Mon, Nov 11, 2024 at 8:15 AM Pavel Tupitsyn <ptupit...@apache.org> > > wrote: > > > > > > Nikita, > > > > > > I see two issues: > > > - Changing the default behavior to a less secure one > > > - Mixing management tasks and user-defined tasks > > > > > > What about a separate client operation code for management tasks? > > > This will provide a way to control this functionality separately, and > > keep > > > the default compute behavior as is. > > > > > > > This is necessary for pluggable commands > > > Are we aware of any extensions that provide their own management > > commands? > > > How does it work with GridClient? Invoke by class name? > > > > > > > > > On Sun, Nov 10, 2024 at 8:40 PM Nikita Amelchev <namelc...@apache.org> > > > wrote: > > > > > > > Hi, Pavel. > > > > > > > > > Which tasks are required for control.sh? > > > > > > > > All management tasks inherit from the `VisorMultiNodeTask` class. > > > > > > > > > Should we add a special case for them instead of enabling all tasks > > by > > > > default? > > > > > > > > We can do it this way, but then it is possible to execute any task in > > > > the Ignite classpath that inherits from the management task class. > > > > This is necessary for pluggable commands - in Ignite extensions and > > > > plugins. > > > > > > > > In any case, to avoid security issues it seems the user should > > > > configure an explicit security plugin - which has the functionality > > > > needed for this. > > > > > > > > On Fri, Nov 8, 2024 at 4:15 PM Pavel Tupitsyn <ptupit...@apache.org> > > > > wrote: > > > > > > > > > > I agree with the proposal in general, but this part is concerning: > > > > > > > > > > > Change the default parameter - > > > > > ThinClientConfiguration#setMaxActiveComputeTasksPerConnection (enable > > > > tasks > > > > > execution by default) > > > > > > > > > > This might become a security issue. > > > > > As a user, I might want to disable thin client compute tasks but > > still > > > > use > > > > > the control utility. > > > > > > > > > > Which tasks are required for control.sh? Should we add a special > > case for > > > > > them instead of enabling all tasks by default? > > > > > > > > > > On Fri, Nov 8, 2024 at 3:00 PM Nikolay Izhikov <nizhi...@apache.org> > > > > wrote: > > > > > > > > > > > +1 > > > > > > > > > > > > > On 8 Nov 2024, at 08:26, Nikita Amelchev <namelc...@apache.org> > > > > wrote: > > > > > > > > > > > > > > Hello, Igniters. > > > > > > > > > > > > > > I propose to migrate the control.sh utility to a thin client. > > This > > > > > > > will allow us to: > > > > > > > > > > > > > > - Remove the legacy GridClient, which is used only for > > control.sh; > > > > > > > - Developers will support one thin client, instead of two; > > > > > > > - Simplify cluster configuration. > > > > > > > > > > > > > > Implementation details and migration notes are described in the > > IEP > > > > > > > [1]. PRs are ready to review [2,3,4]. Please, take a look. > > > > > > > > > > > > > > [1] > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/IGNITE/%5BPhase+3%5D+Use+IgniteClient+instead+of+GridClient+in+the+control.sh+utility > > > > > > > [2] https://github.com/apache/ignite/pull/11647 > > > > > > > [3] https://github.com/apache/ignite/pull/11646 > > > > > > > [4] https://github.com/apache/ignite/pull/11648 > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > Best wishes, > > > > > > > Amelchev Nikita > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > Best wishes, > > > > Amelchev Nikita > > > > > > > > > > > > -- > > Best wishes, > > Amelchev Nikita > > -- Best wishes, Amelchev Nikita
