Re: Review for IGNITE-13112 The current security context should be obtained using the IgniteSecurity interface only.
Maksim, ok. Let me know if you have any questions. ср, 21 апр. 2021 г. в 17:51, Maksim Stepachev : > Please wait. I'm watching your review. > > вт, 6 апр. 2021 г. в 20:14, Denis Garus : > > > Hello, Igniters! > > > > I've raised the PR [1] for the issue [2]. > > Could somebody review it? > > > > Suggested implementation > > > > If Ignite Security (IS) is enabled, then executors, accessed through the > > PoolProcessor, > > are wrapped to a security-aware implementation. Security-aware > > implementation sets proper > > security context for tasks that the executor performs. > > > > The field subject id was deleted from communication requests for cache > and > > compute operations; > > a remote node gets the subject id that initiates the ignite operation > from > > GridIoSecurityAwareMessage. > > IgniteSecurity uses this id to set a proper security context during the > > execution of the request. > > > > Remove GridTaskThreadContextKey#TC_SUBJ_ID, > > GridCacheContext#subjectIdPerCall; > > a consumer has to obtain a current security subject id through > > IgniteSecurity > > or the set of SecurityUtils methods. > > > > For all events that include the subject id field, are set the following > > rule. > > If IS is enabled, this field must contain a subject id that initiates > > an ignite operation, otherwise null. > > > > Implement SecurityAwareCustomMessageWrapper for discovery requests that > act > > as > > GridIoSecurityAwareMessage for communication requests. It allows setting > > proper > > context during the discovery message execution. > > > > Implement SecurityAwareGridRestCommandHandler to allow GridRestProcessor > > to execute all client requests with the proper security context. > > > > 1. https://github.com/apache/ignite/pull/8038 > > 2. https://issues.apache.org/jira/browse/IGNITE-13112 > > >
Re: Model of permissions for Ignite 3
Hello! We arranged that I prepare an IEP. If you have some ideas about what should be reflected in this IEP, do not hesitate to let me know. вт, 20 апр. 2021 г. в 02:06, Valentin Kulichenko < valentin.kuliche...@gmail.com>: > Hi folks, > > Did you have a discussion? How did it go? Can someone summarize the > results? > > -Val > > On Mon, Apr 12, 2021 at 2:00 AM Kseniya Romanova < > romanova.ks@gmail.com> > wrote: > > > Hi! Scheduled open call for Friday > > https://www.meetup.com/Moscow-Apache-Ignite-Meetup/events/277518672/ > > Please join to see the zoom link. Consulted with Denis Garus and put the > > topic as "Security", cause it's definitely wider than just permissions. > > > > Cheers! > > > > пн, 12 апр. 2021 г. в 09:25, Alexei Scherbakov < > > alexey.scherbak...@gmail.com > > >: > > > > > +1 > > > > > > We should rethink the security model in Ignite 3 and have a default > RBAC > > > based implementation, from my point of view. > > > By default, no code should be written to enable and use it. > > > > > > Let's schedule a meeting and discuss the details. > > > > > > вс, 11 апр. 2021 г. в 19:43, Denis Garus : > > > > > > > Andrey, Alexey thank you for the feedback! > > > > > > > > > I suggest decoupling authentication from authorization > > > > > > > > Yes, it would be useful. Existing SecuritySubject and SecurityContext > > > > require reworking. > > > > > > > > > I think it would be great to have a default implementation of > > > > user-role-permission model > > > > > > > > Completely agree it is a cool idea. Ignite should have more default > > > > implementation referred to security. > > > > > > > > > Should we have a community meeting to discuss this? > > > > > > > > Yes, it would be great. > > > > The wish list for Ignite 3 does not contain security improvement > that, > > > > IMHO, is wrong. > > > > We should fix that oversight on early-stage Ignite 3 development. > > > > > > > > пт, 9 апр. 2021 г. в 18:47, Alexey Goncharuk < > > alexey.goncha...@gmail.com > > > >: > > > > > > > > > Hello Denis, Andrey, Igniters, > > > > > > > > > > Why don't we take a step further in improving the security model in > > > > Ignite > > > > > 3? I think it would be great to have a default implementation of > > > > > user-role-permission model in Ignite to be on par with security > > models > > > of > > > > > widely-used databases. This will complement community efforts in > > making > > > > > most of the Ignite 3 behavior to be changeable in runtime. > > > > > > > > > > WDYT? Should we have a community meeting to discuss this? > > > > > > > > > > > > > > > чт, 8 апр. 2021 г. в 23:54, Andrey Kuznetsov : > > > > > > > > > > > Hi Denis! > > > > > > > > > > > > The idea and prototype look great. > > > > > > > > > > > > I'd like to highlight one arguable point. Default authorization > > > > > > implementation still assumes there are permissions provided in > > > > > > SecuritySubject. In turn, authentication is still responsible for > > > > filling > > > > > > these permissions. I suggest decoupling authentication from > > > > > authorization, > > > > > > so that GridSecurityProcessor implementation is fully responsible > > for > > > > > > obtaining permissions for SecuritySubject given on authorization. > > In > > > > > > particular, implementation can choose an existing behavior of > > > bundling > > > > > > permissions with SecuritySubject. > > > > > > > > > > > > Makes sense? > > > > > > > > > > > > чт, 8 апр. 2021 г. в 17:52, Denis Garus : > > > > > > > > > > > > > Sorry, I forgot to point the link > > > > > > > > > > > > > > 1. https://github.com/apache/ignite/pull/8989 > > > > > > > > > > > > > > чт, 8 апр. 2021 г. в 17:50, Denis Garus : > > > > > > > > > > > > > >
Re: Model of permissions for Ignite 3
Andrey, Alexey thank you for the feedback!
> I suggest decoupling authentication from authorization
Yes, it would be useful. Existing SecuritySubject and SecurityContext
require reworking.
> I think it would be great to have a default implementation of
user-role-permission model
Completely agree it is a cool idea. Ignite should have more default
implementation referred to security.
> Should we have a community meeting to discuss this?
Yes, it would be great.
The wish list for Ignite 3 does not contain security improvement that,
IMHO, is wrong.
We should fix that oversight on early-stage Ignite 3 development.
пт, 9 апр. 2021 г. в 18:47, Alexey Goncharuk :
> Hello Denis, Andrey, Igniters,
>
> Why don't we take a step further in improving the security model in Ignite
> 3? I think it would be great to have a default implementation of
> user-role-permission model in Ignite to be on par with security models of
> widely-used databases. This will complement community efforts in making
> most of the Ignite 3 behavior to be changeable in runtime.
>
> WDYT? Should we have a community meeting to discuss this?
>
>
> чт, 8 апр. 2021 г. в 23:54, Andrey Kuznetsov :
>
> > Hi Denis!
> >
> > The idea and prototype look great.
> >
> > I'd like to highlight one arguable point. Default authorization
> > implementation still assumes there are permissions provided in
> > SecuritySubject. In turn, authentication is still responsible for filling
> > these permissions. I suggest decoupling authentication from
> authorization,
> > so that GridSecurityProcessor implementation is fully responsible for
> > obtaining permissions for SecuritySubject given on authorization. In
> > particular, implementation can choose an existing behavior of bundling
> > permissions with SecuritySubject.
> >
> > Makes sense?
> >
> > чт, 8 апр. 2021 г. в 17:52, Denis Garus :
> >
> > > Sorry, I forgot to point the link
> > >
> > > 1. https://github.com/apache/ignite/pull/8989
> > >
> > > чт, 8 апр. 2021 г. в 17:50, Denis Garus :
> > >
> > > > Hello, Igniters!
> > > >
> > > > I want to propose to improve the way which we use
> > > > to present permissions in Ignite 3.
> > > >
> > > > The model of permission in Ignite has a set of drawbacks.
> > > > The main drawback, IMHO: if you need to add a new permission,
> > > > you should change the core module by extended the
> 'SecurityPermission'
> > > > enum.
> > > > An approach like this becomes more challenged if new permission is
> > > created
> > > > for an extension.
> > > >
> > > > The existing permission model is overcomplicated.
> > > > The SecurityPermission enum is divided into four groups,
> > > > and to determine whether a security subject has been given
> permission,
> > > > a plugin developer has to know what the permission group is.
> > > > But 'CACHE_CREATE' and 'CACHE_DESTROY' are included in two groups
> > (system
> > > > operations and cache operations).
> > > > When 'CACHE_CREATE' ('CACHE_DESTROY') is treated as system
> permission,
> > > > it applies to all caches. In other cases, when 'CACHE_CREATE'
> > > > ('CACHE_DESTROY') is treated as cache permission,
> > > > permission checking is executed with the account of the cache name.
> > > > IMHO, this logic is hard to understand.
> > > > There is no ability to represent compound operation as single
> > permission
> > > > and so on.
> > > >
> > > >
> > > > So I would like to suggest using a permission model that is based on
> > > > 'java.security.Permission'.
> > > > I prepared the concept [1] of how this model could look in Ignite.
> > > > Classes 'CachePermission', 'ComputePermission', and
> 'ServicePermission'
> > > > represent cache, compute,
> > > > and service permissions accordingly, allow wildcards, for example,
> > > > "org.apache.ignite.internal.*".
> > > > Class 'IgniteClusterPermission' represents permission without
> actions.
> > > > Interface 'GridSecurityProcessor' has a default implementation of the
> > > > 'authorize' method.
> > > > 'SecurityTestSuite' is green.
> > > >
> > > >
> > > > This representation of permission, IMHO, has the following
> advantages:
> > > > - A developer can easily add new permission without needing to touch
> > the
> > > > core module.
> > > > - There is no need to implement complicated logic to authorize an
> > > > operation inside a security plugin.
> > > >But a developer has the opportunity to add custom logic.
> > > > - Wildcards for permission's name from a box, for example, 'new
> > > > CachePermission("x.y.z.*", "get,put")'.
> > > > - There is no need to implement 'SecurityPermissionSet' and a set of
> > > > methods from 'SecurityContex' ('xxxAllowed(String,
> > SecurityPermission))'.
> > > > - We can define a security policy in a file as java does. It could
> > > > simplify work for administrators.
> > > >
> > > > WDYT?
> > > >
> > >
> >
> >
> > --
> > Best regards,
> > Andrey Kuznetsov.
> >
>
Re: Model of permissions for Ignite 3
Sorry, I forgot to point the link
1. https://github.com/apache/ignite/pull/8989
чт, 8 апр. 2021 г. в 17:50, Denis Garus :
> Hello, Igniters!
>
> I want to propose to improve the way which we use
> to present permissions in Ignite 3.
>
> The model of permission in Ignite has a set of drawbacks.
> The main drawback, IMHO: if you need to add a new permission,
> you should change the core module by extended the 'SecurityPermission'
> enum.
> An approach like this becomes more challenged if new permission is created
> for an extension.
>
> The existing permission model is overcomplicated.
> The SecurityPermission enum is divided into four groups,
> and to determine whether a security subject has been given permission,
> a plugin developer has to know what the permission group is.
> But 'CACHE_CREATE' and 'CACHE_DESTROY' are included in two groups (system
> operations and cache operations).
> When 'CACHE_CREATE' ('CACHE_DESTROY') is treated as system permission,
> it applies to all caches. In other cases, when 'CACHE_CREATE'
> ('CACHE_DESTROY') is treated as cache permission,
> permission checking is executed with the account of the cache name.
> IMHO, this logic is hard to understand.
> There is no ability to represent compound operation as single permission
> and so on.
>
>
> So I would like to suggest using a permission model that is based on
> 'java.security.Permission'.
> I prepared the concept [1] of how this model could look in Ignite.
> Classes 'CachePermission', 'ComputePermission', and 'ServicePermission'
> represent cache, compute,
> and service permissions accordingly, allow wildcards, for example,
> "org.apache.ignite.internal.*".
> Class 'IgniteClusterPermission' represents permission without actions.
> Interface 'GridSecurityProcessor' has a default implementation of the
> 'authorize' method.
> 'SecurityTestSuite' is green.
>
>
> This representation of permission, IMHO, has the following advantages:
> - A developer can easily add new permission without needing to touch the
> core module.
> - There is no need to implement complicated logic to authorize an
> operation inside a security plugin.
>But a developer has the opportunity to add custom logic.
> - Wildcards for permission's name from a box, for example, 'new
> CachePermission("x.y.z.*", "get,put")'.
> - There is no need to implement 'SecurityPermissionSet' and a set of
> methods from 'SecurityContex' ('xxxAllowed(String, SecurityPermission))'.
> - We can define a security policy in a file as java does. It could
> simplify work for administrators.
>
> WDYT?
>
Model of permissions for Ignite 3
Hello, Igniters!
I want to propose to improve the way which we use
to present permissions in Ignite 3.
The model of permission in Ignite has a set of drawbacks.
The main drawback, IMHO: if you need to add a new permission,
you should change the core module by extended the 'SecurityPermission'
enum.
An approach like this becomes more challenged if new permission is created
for an extension.
The existing permission model is overcomplicated.
The SecurityPermission enum is divided into four groups,
and to determine whether a security subject has been given permission,
a plugin developer has to know what the permission group is.
But 'CACHE_CREATE' and 'CACHE_DESTROY' are included in two groups (system
operations and cache operations).
When 'CACHE_CREATE' ('CACHE_DESTROY') is treated as system permission,
it applies to all caches. In other cases, when 'CACHE_CREATE'
('CACHE_DESTROY') is treated as cache permission,
permission checking is executed with the account of the cache name.
IMHO, this logic is hard to understand.
There is no ability to represent compound operation as single permission
and so on.
So I would like to suggest using a permission model that is based on
'java.security.Permission'.
I prepared the concept [1] of how this model could look in Ignite.
Classes 'CachePermission', 'ComputePermission', and 'ServicePermission'
represent cache, compute,
and service permissions accordingly, allow wildcards, for example,
"org.apache.ignite.internal.*".
Class 'IgniteClusterPermission' represents permission without actions.
Interface 'GridSecurityProcessor' has a default implementation of the
'authorize' method.
'SecurityTestSuite' is green.
This representation of permission, IMHO, has the following advantages:
- A developer can easily add new permission without needing to touch the
core module.
- There is no need to implement complicated logic to authorize an operation
inside a security plugin.
But a developer has the opportunity to add custom logic.
- Wildcards for permission's name from a box, for example, 'new
CachePermission("x.y.z.*", "get,put")'.
- There is no need to implement 'SecurityPermissionSet' and a set of
methods from 'SecurityContex' ('xxxAllowed(String, SecurityPermission))'.
- We can define a security policy in a file as java does. It could simplify
work for administrators.
WDYT?
Review for IGNITE-13112 The current security context should be obtained using the IgniteSecurity interface only.
Hello, Igniters! I've raised the PR [1] for the issue [2]. Could somebody review it? Suggested implementation If Ignite Security (IS) is enabled, then executors, accessed through the PoolProcessor, are wrapped to a security-aware implementation. Security-aware implementation sets proper security context for tasks that the executor performs. The field subject id was deleted from communication requests for cache and compute operations; a remote node gets the subject id that initiates the ignite operation from GridIoSecurityAwareMessage. IgniteSecurity uses this id to set a proper security context during the execution of the request. Remove GridTaskThreadContextKey#TC_SUBJ_ID, GridCacheContext#subjectIdPerCall; a consumer has to obtain a current security subject id through IgniteSecurity or the set of SecurityUtils methods. For all events that include the subject id field, are set the following rule. If IS is enabled, this field must contain a subject id that initiates an ignite operation, otherwise null. Implement SecurityAwareCustomMessageWrapper for discovery requests that act as GridIoSecurityAwareMessage for communication requests. It allows setting proper context during the discovery message execution. Implement SecurityAwareGridRestCommandHandler to allow GridRestProcessor to execute all client requests with the proper security context. 1. https://github.com/apache/ignite/pull/8038 2. https://issues.apache.org/jira/browse/IGNITE-13112
Re: [DISCUSSION] Merge APIs of IgniteAuthenticationProcessor and IgniteSecurity
Hello, Mikhail! Proposed realization provides a security plugin when isAuthenticationEnabled is true and, in this way, makes IgniteSecurity enabled. But current implementation of IgniteAuthenticationProcessor (security plugin) does not allow: - apply a security policy, so authorization does not make sense - authenticate nodes - get the actual security context by a security subject id. Another hand, security-enabled mode adds to every communication message a security subject id, makes a security context switch, authorizes an operation, so these all look like a waste of resources. IMHO a better solution would be to implement a full-functional default security plugin. пн, 22 мар. 2021 г. в 15:52, Mikhail Petrov : > Maxim, this issue should be fixed as part of [1]. Note, that the current > PR [2] is draft and its description just shows the current state of > work. Of course the task i'm working on can't be resolved without fixing > this issue. > > [1] - https://issues.apache.org/jira/browse/IGNITE-13112 > [2] - https://github.com/apache/ignite/pull/8892 > > On 22.03.2021 15:38, Maxim Muzafarov wrote: > > Mikhail, > > > >> Note, that the current PR breaks management of the users via REST > because the SecurityContext is not propagated properly during REST requests > execution. > > Do we have a JIRA taks to fix this behaviour or do we need such > > behaviour at all? > > > > On Mon, 22 Mar 2021 at 13:34, Nikolay Izhikov > wrote: > >> Hello, Mikhail. > >> > >> I'm +1 to follow your suggestion. > >> > >> чт, 18 мар. 2021 г. в 17:53, Mikhail Petrov : > >> > >>> Hello, Igniters. > >>> > >>> As of now, there are two independent APIs related to security: > >>> 1. IgniteSecurity - handle node/client authentication and authorize all > >>> operations. > >>> 2. IgniteAuthenticationProcessor - handle authentication of thin > clients > >>> only. > >>> > >>> The main purpose of creating the IgniteAuthenticationProcessor was to > >>> bring default security implementation in Ignite (see [1]) because > >>> IgniteSecurity has always had a single implementation that delegates > >>> authorization and authentication operation to an external security > plugin. > >>> > >>> But two different APIs that are related to the same leads to security > >>> checks duplication in code. As of now, it's possible to configure both > >>> security approaches at the same time, and that is confusing for the > >>> user. E.g., the user can provide a security plugin and execute ALTER / > >>> DROP / CREATE commands successfully. In this case, mentioned commands > >>> will do nothing because they only work with the authentication > processor > >>> > >>> I propose to merge the two mentioned above security APIs into one based > >>> on the IgniteSecurity interface. > >>> > >>> For this it is proposed: > >>> 1. Remove an IgniteAuthenticationProcessor that is now treated as an > >>> independent processor. > >>> 2. Move the logic of IgniteAuthenticationProcessor into the > >>> implementation of the security plugin that will be used if > >>> authentication is enabled via configuration. > >>> 3. Remove duplication of security checks and leave IgniteSecurity as a > >>> single security API. As of now, authentication operations are not > >>> delegated to IgniteAuthenticationProcessor if a security plugin is > >>> specified. So the overall security behavior from the user side will > >>> remain intact. > >>> 4. Remove the AuthorizationContext completely as IgniteSecurity > provides > >>> an API for storing and managing the security contexts. > >>> 5. Extend GridSecurityPlugin interface with methods that provide the > >>> ability to manage security users to support existing commands available > >>> for authentication processor - alter/drop/create through SQL and REST. > >>> > >>> As a result, we will make the security-related code more consistent and > >>> simpler. > >>> > >>> Proposed signatures of GridSecurityPlugin methods that provide the > >>> ability to manage security users: > >>> > >>> public void createUser(String login, UserOptions opts) throws > >>> IgniteCheckedException  > >>>  > >>> public void alterUser(String login, UserOptions opts) throws > >>> IgniteCheckedException > >>>  > >>> public void dropUser(String login) throws IgniteCheckedException > >>> > >>> The UserOptions class is a wrapper over EnumMap that maps option values > >>> to their aliases. This allows the class to be used for both create > >>> and alter user operations and doesn't break backward compatibility in > >>> case new options are declared. > >>> > >>>  > >>> The proposed changes lead to the following compatibility issues: > >>> > >>> 1. When a user provides a security plugin and enables authentication - > >>> in this case, the user will face exceptions during the node start while > >>> now node starts smoothly. This case makes a little sense because now > >>> authentication operations are not delegated to > >>>
Re: [DISCUSSION] IgniteFuture class future in Ignite-3.0.
> Stripping them from such functionality, which they are used to, is most
likely a bad idea.
Completely agree with this point of view.
Moreover, a user can pass CompletableFuture to another library to do any
manipulations.
So if we want to introduce our class instead of the java class, we should
have solid arguments;
otherwise, it can be a reason for irritation.
ср, 31 мар. 2021 г. в 09:06, Pavel Tupitsyn :
> Val,
>
> > we can have an IgniteFuture that extends CompletableFuture.
> > This might be useful if want the cancel() operation to cancel the
> > underlying operation
>
> I think we can subscribe to the cancellation of the CompletableFuture
> and cancel the underlying operation without an extra class,
> something like
>
> fut.exceptionally(t -> {
> if (t instanceof CancellationException) {
> // Cancel Ignite operation
> }
> });
>
> On Wed, Mar 31, 2021 at 7:45 AM Valentin Kulichenko <
> valentin.kuliche...@gmail.com> wrote:
>
> > These are actually some interesting points. As I'm thinking more about
> > this, I'm leaning towards changing my opinion and voting for the
> > CompletableFuture. Here is my reasoning.
> >
> > First, it's important to keep in mind that CompletableFuture is not an
> > interface that we will implement, it's an implemented class. Therefore,
> > some of the concerns around complete() and cancel() method are not really
> > relevant -- it's not up to us how these methods behave, they're already
> > implemented.
> >
> > Second, CompletableFuture does provide some useful functionality (anyOf
> is
> > one of the examples). I can even envision users wanting to complete the
> > future under certain circumstances, e.g. after a timeout, using
> > the completeOnTimeout method. Stripping them from such functionality,
> which
> > they are used to, is most likely a bad idea.
> >
> > And finally, we can have an IgniteFuture that extends CompletableFuture.
> > This might be useful if want the cancel() operation to cancel the
> > underlying operation. This way we keep all the functionality of
> > CompletableFuture while keeping a certain amount of flexibility for
> > specific cases.
> >
> > Thoughts?
> >
> > -Val
> >
> >
> > On Tue, Mar 30, 2021 at 5:36 AM Denis Garus wrote:
> >
> > > > Completing future from outside will never respect other subscribers
> > that
> > > > may expect other guatantees.
> > >
> > > For example, if we talk about public API like IgniteCache, what
> > subscribers
> > > may expect other guatantees?
> > > IMHO, the best solution is to get the well-known standard interface to
> a
> > > user, and he will be happy.
> > >
> > > But when we talk about internal classes like "exchange future" they
> could
> > > be custom futures if convenient.
> > >
> > > вт, 30 мар. 2021 г. в 15:25, Atri Sharma :
> > >
> > > > IMO the only way Ignite should cancel computations is iff cancel
> method
> > > is
> > > > invoked, not implicitly if complete is invoked.
> > > >
> > > > On Tue, 30 Mar 2021 at 4:58 PM, Denis Garus
> > wrote:
> > > >
> > > > > Hello!
> > > > >
> > > > > > Let's say a user started a compute with fut =
> > compute.runAsync(task);
> > > > > > and now calls fut.complete(someVal); Does this mean that Ignite
> no
> > > > longer
> > > > > needs to execute the task?
> > > > > > If the task is currently running, does it need to be canceled?
> > > > >
> > > > > Yes, this case looks like Ignite should cancel computations
> because a
> > > > user
> > > > > wants to complete the future. Why not?
> > > > > If there will be an opportunity to cancel a future, why is it a bad
> > > > option
> > > > > to finish a future through a complete() method?
> > > > >
> > > > > > If you look at Ignite-2 code, you may found a number of places
> > where
> > > we
> > > > > return e.g. exchange futures or partition release futures.
> > > > > > Assume the impact if we will return CompletableFuture instead,
> > which
> > > > can
> > > > > be completed in 3-rd party plugin by mistake?
> > > > >
> > > > > If exchange futures or partition release futures can be returned to
> > > 3-rd
> &g
Re: [DISCUSSION] IgniteFuture class future in Ignite-3.0.
> Completing future from outside will never respect other subscribers that > may expect other guatantees. For example, if we talk about public API like IgniteCache, what subscribers may expect other guatantees? IMHO, the best solution is to get the well-known standard interface to a user, and he will be happy. But when we talk about internal classes like "exchange future" they could be custom futures if convenient. вт, 30 мар. 2021 г. в 15:25, Atri Sharma : > IMO the only way Ignite should cancel computations is iff cancel method is > invoked, not implicitly if complete is invoked. > > On Tue, 30 Mar 2021 at 4:58 PM, Denis Garus wrote: > > > Hello! > > > > > Let's say a user started a compute with fut = compute.runAsync(task); > > > and now calls fut.complete(someVal); Does this mean that Ignite no > longer > > needs to execute the task? > > > If the task is currently running, does it need to be canceled? > > > > Yes, this case looks like Ignite should cancel computations because a > user > > wants to complete the future. Why not? > > If there will be an opportunity to cancel a future, why is it a bad > option > > to finish a future through a complete() method? > > > > > If you look at Ignite-2 code, you may found a number of places where we > > return e.g. exchange futures or partition release futures. > > > Assume the impact if we will return CompletableFuture instead, which > can > > be completed in 3-rd party plugin by mistake? > > > > If exchange futures or partition release futures can be returned to 3-rd > > party plugin by mistake, it is poor encapsulation. > > And if it will be IgniteFuter rather than CompletedFuture, anyway, this > can > > harm. > > > > вт, 30 мар. 2021 г. в 13:14, Andrey Mashenkov < > andrey.mashen...@gmail.com > > >: > > > > > Guys, > > > > > > I want to remember there is one more point to pay attention to. > > > Extending Future and CompletableStage is more than just prevents > > unexpected > > > behavior if a user completed the future. > > > > > > First of all, it helps us to write safer code as we won't a method > > contract > > > exposed such methods as to a user as to a developer. > > > If you look at Ignite-2 code, you may found a number of places where we > > > return e.g. exchange futures or partition release futures. > > > Assume the impact if we will return CompletableFuture instead, which > can > > be > > > completed in 3-rd party plugin by mistake? > > > > > > The suggested approach allows us to don't bother if a CompletableFuture > > has > > > to be wrapped or not. > > > > > > > > > On Tue, Mar 30, 2021 at 12:22 PM Alexey Goncharuk < > > > alexey.goncha...@gmail.com> wrote: > > > > > > > Ivan, > > > > > > > > My concern with the concept of a user completing the future returned > > from > > > > Ignite public API is that it is unclear how to interpret this action > > > (this > > > > backs Val's message). > > > > Let's say a user started a compute with fut = compute.runAsync(task); > > and > > > > now calls fut.complete(someVal); Does this mean that Ignite no longer > > > needs > > > > to execute the task? If the task is currently running, does it need > to > > be > > > > canceled? > > > > > > > > Using CompletableFuture.anyOf() is a good instrument in this case > > because > > > > it makes the 'first future wins' contract explicit in the code. > Besides > > > > that, the point regarding the cancel() method is valid, and we will > > need > > > > some custom mechanics to cancel a computation, so a custom interface > > that > > > > simply extends both Future and CompletableStage seems reasonable to > me. > > > > > > > > --AG > > > > > > > > пн, 29 мар. 2021 г. в 09:12, Ivan Pavlukhin : > > > > > > > > > Val, > > > > > > > > > > There were enough hype around Reactive programming past years. I > > > > > remind a lot of talks about RxJava. And I suppose it worth to > > consider > > > > > it. But it requires some time to study modern trends to make a > > choice. > > > > > So far I am not ready to facilitate Reactive API for Ignite 3. > > > > > > > > > > Regarding CompletableFuture. > > > > >
Re: [DISCUSSION] IgniteFuture class future in Ignite-3.0.
Hello!
> Let's say a user started a compute with fut = compute.runAsync(task);
> and now calls fut.complete(someVal); Does this mean that Ignite no longer
needs to execute the task?
> If the task is currently running, does it need to be canceled?
Yes, this case looks like Ignite should cancel computations because a user
wants to complete the future. Why not?
If there will be an opportunity to cancel a future, why is it a bad option
to finish a future through a complete() method?
> If you look at Ignite-2 code, you may found a number of places where we
return e.g. exchange futures or partition release futures.
> Assume the impact if we will return CompletableFuture instead, which can
be completed in 3-rd party plugin by mistake?
If exchange futures or partition release futures can be returned to 3-rd
party plugin by mistake, it is poor encapsulation.
And if it will be IgniteFuter rather than CompletedFuture, anyway, this can
harm.
вт, 30 мар. 2021 г. в 13:14, Andrey Mashenkov :
> Guys,
>
> I want to remember there is one more point to pay attention to.
> Extending Future and CompletableStage is more than just prevents unexpected
> behavior if a user completed the future.
>
> First of all, it helps us to write safer code as we won't a method contract
> exposed such methods as to a user as to a developer.
> If you look at Ignite-2 code, you may found a number of places where we
> return e.g. exchange futures or partition release futures.
> Assume the impact if we will return CompletableFuture instead, which can be
> completed in 3-rd party plugin by mistake?
>
> The suggested approach allows us to don't bother if a CompletableFuture has
> to be wrapped or not.
>
>
> On Tue, Mar 30, 2021 at 12:22 PM Alexey Goncharuk <
> alexey.goncha...@gmail.com> wrote:
>
> > Ivan,
> >
> > My concern with the concept of a user completing the future returned from
> > Ignite public API is that it is unclear how to interpret this action
> (this
> > backs Val's message).
> > Let's say a user started a compute with fut = compute.runAsync(task); and
> > now calls fut.complete(someVal); Does this mean that Ignite no longer
> needs
> > to execute the task? If the task is currently running, does it need to be
> > canceled?
> >
> > Using CompletableFuture.anyOf() is a good instrument in this case because
> > it makes the 'first future wins' contract explicit in the code. Besides
> > that, the point regarding the cancel() method is valid, and we will need
> > some custom mechanics to cancel a computation, so a custom interface that
> > simply extends both Future and CompletableStage seems reasonable to me.
> >
> > --AG
> >
> > пн, 29 мар. 2021 г. в 09:12, Ivan Pavlukhin :
> >
> > > Val,
> > >
> > > There were enough hype around Reactive programming past years. I
> > > remind a lot of talks about RxJava. And I suppose it worth to consider
> > > it. But it requires some time to study modern trends to make a choice.
> > > So far I am not ready to facilitate Reactive API for Ignite 3.
> > >
> > > Regarding CompletableFuture.
> > >
> > > > The point is that currently a future returned from any of Ignite's
> > async
> > > > operations is supposed to be completed with a value only by Ignite
> > > itself,
> > > > not by the user. If we follow the same approach in Ignite 3,
> returning
> > > > CompletableFuture is surely wrong in my view.
> > >
> > > My first thoughts was similar. But later I thought what a user would
> > > like do with returned future. And one of cases I imagined was a case
> > > of alternative result. E.g. a user uses Ignite and another data source
> > > in his application. He wants to use a value arrived faster. He
> > > combines 2 futures like CompletableFuture.anyOf(...). Consequently
> > > even if we prohibit CompletableFuture.complete(...) explicitly then it
> > > will be possible to create a combination that will allow premature
> > > future completion. After all generally CompletableFuture is a
> > > placeholder for async computaion result and if a user wants to
> > > substitute result returned from Ignite why should we disallow him to
> > > do it?
> > >
> > > Also I found one more suspicious thing with CompletableFuture. As it
> > > is a concrete class it implements a cancel() method. And as I see the
> > > implementation does not try to cancel underlying computations. Is not
> > > it a problem?
> > >
> > > 2021-03-29 7:30 GMT+03:00, Valentin Kulichenko <
> > > valentin.kuliche...@gmail.com>:
> > > > Ivan,
> > > >
> > > > It's not really about the "harm", but more about "what should we do
> if
> > > this
> > > > method is called?". Imagine the following code:
> > > >
> > > > CompletableFuture fut = cache.getAsync(key);
> > > > fut.complete("something");
> > > >
> > > > What should happen in this case?
> > > >
> > > > The point is that currently a future returned from any of Ignite's
> > async
> > > > operations is supposed to be completed with a value only by Ignite
> > > itself,
> > > > not by the user. If we
Re: IEP-70: Async Continuation Executor
Pavel,
I tried this:
@Test
public void test() throws Exception {
IgniteCache cache =
startGrid().getOrCreateCache("test_cache");
cache.putAsync(1, "one").listen(f -> cache.replace(1, "two"));
assertEquals("two", cache.get(1));
}
and this test is green.
I believe that an user can make listener that leads to deadlock, but
the example in the IEP does not reflect this.
ср, 17 мар. 2021 г. в 17:36, Вячеслав Коптилин :
> Hi Pavel,
>
> > Not a good excuse really. We have a usability problem, you have to admit
> it.
> Fair enough. I agree that this is a usability issue, but I have doubts that
> the proposed approach to overcome it is the best one.
>
> > Documentation won't help - no one is going to read the Javadoc for a
> trivial method like putAsync
> That is sad... However, I don't think that this is a strong argument here.
>
> This is just my opinion. Let's see what other community members have to
> say.
>
> Thanks,
> S.
>
>
> ср, 17 мар. 2021 г. в 17:01, Pavel Tupitsyn :
>
> > > the user should use the right API
> >
> > Not a good excuse really. We have a usability problem, you have to admit
> > it.
> > "The brakes did not work on your car - too bad, you should have known
> that
> > on Sundays only your left foot is allowed on the pedal"
> >
> > This particular use case is too intricate.
> > Even when you know about that, it is difficult to decide what can run on
> > the striped pool,
> > and what can't. It is too easy to forget.
> > And most people don't know, even among Ignite developers.
> >
> > Documentation won't help - no one is going to read the Javadoc for a
> > trivial method like putAsync.
> >
> >
> > So I propose to have a safe default.
> > Then document the performance tuning opportunity on [1].
> >
> > Think about how many users abandon a product because it mysteriously
> > crashes and hangs.
> >
> > [1]
> >
> >
> https://ignite.apache.org/docs/latest/perf-and-troubleshooting/general-perf-tips
> >
> >
> > On Wed, Mar 17, 2021 at 4:21 PM Вячеслав Коптилин <
> > slava.kopti...@gmail.com>
> > wrote:
> >
> > > Hi Pavel,
> > >
> > > Well, I think that the user should use the right API instead of
> > introducing
> > > uncontested overhead for everyone.
> > > For instance, the code that is provided by IEP can changed as follows:
> > >
> > > IgniteFuture fut = cache.putAsync(1, 1);
> > > fut.listenAync(f -> {
> > > // Executes on Striped pool and deadlocks.
> > > cache.replace(1, 2);
> > > }, ForkJoinPool.commonPool());
> > >
> > > Of course, it does not mean that this fact should not be properly
> > > documented.
> > > Perhaps, I am missing something.
> > >
> > > Thanks,
> > > S.
> > >
> > > ср, 17 мар. 2021 г. в 16:01, Pavel Tupitsyn :
> > >
> > > > Slava,
> > > >
> > > > Your suggestion is to keep things as is and discard the IEP, right?
> > > >
> > > > > this can lead to significant overhead
> > > > Yes, there is some overhead, but the cost of accidentally starving
> the
> > > > striped pool is worse,
> > > > not to mention the deadlocks.
> > > >
> > > > I believe that we should favor correctness over performance in any
> > case.
> > > >
> > > >
> > > > On Wed, Mar 17, 2021 at 3:34 PM Вячеслав Коптилин <
> > > > slava.kopti...@gmail.com>
> > > > wrote:
> > > >
> > > > > Well, the specified method already exists :)
> > > > >
> > > > > /**
> > > > > * Registers listener closure to be asynchronously notified
> > > whenever
> > > > > future completes.
> > > > > * Closure will be processed in specified executor.
> > > > > *
> > > > > * @param lsnr Listener closure to register. Cannot be {@code
> > > null}.
> > > > > * @param exec Executor to run listener. Cannot be {@code
> null}.
> > > > > */
> > > > > public void listenAsync(IgniteInClosure IgniteFuture>
> > > > lsnr,
> > > > > Executor exec);
> > > > >
> > > > > Thanks,
> > > > > S.
> > > > >
> > > > > ср, 17 мар. 2021 г. в 15:25, Вячеслав Коптилин <
> > > slava.kopti...@gmail.com
> > > > >:
> > > > >
> > > > > > Hello Pavel,
> > > > > >
> > > > > > I took a look at your IEP and pool request. I have the following
> > > > > concerns.
> > > > > > First of all, this change breaks the contract of
> > > > > IgniteFuture#listen(lsnr)
> > > > > >
> > > > > > /**
> > > > > > * Registers listener closure to be asynchronously notified
> > > > whenever
> > > > > > future completes.
> > > > > > * Closure will be processed in thread that completes this
> > future
> > > > or
> > > > > > (if future already
> > > > > > * completed) immediately in current thread.
> > > > > > *
> > > > > > * @param lsnr Listener closure to register. Cannot be {@code
> > > > null}.
> > > > > > */
> > > > > > public void listen(IgniteInClosure>
> > > lsnr);
> > > > > >
> > > > > > In your pull request, the listener is always called from a
> > > > specified
> > > > > > thread pool (which is fork-join by default)
> > > > > > even though the future is already completed at the
Re: IEP-70: Async Continuation Executor
Pavel, I got it. Thank you. ср, 17 мар. 2021 г. в 13:11, Pavel Tupitsyn : > Denis, > > Yes, I've updated the Motivation section, it really was a bit vague - thank > you. > > > why you suggest using ForkJoinPool.commonPool as the default pool to run > listeners > > - It is recommended to be used by default [1] > - There are no alternatives? > > [1] > > https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ForkJoinPool.html > > On Wed, Mar 17, 2021 at 1:02 PM Denis Garus wrote: > > > Pavel, thank you for your answer. > > > > Sorry, the previous version of IEP motivation highlighted what could be > > wrong with user listeners, > > so I thought that is the main problem. > > > > I'm wondering why you suggest using ForkJoinPool.commonPool as the > default > > pool to run listeners? > > > > ср, 17 мар. 2021 г. в 11:51, Alexey Kukushkin >: > > > > > My initial confusion was due to I did not know the > > > TaskCompletionSource.SetResult() internally handles > > > SynchronizationContext captured before the async operation. I thought > we > > > would have to do it in Ignite. Now I know that and have no problems > with > > > the IEP. > > > > > > ср, 17 мар. 2021 г. в 11:18, Pavel Tupitsyn : > > > > > > > Denis, > > > > > > > > > we don't try to solve the problem mentioned in the IEP > > > > > > > > The problem: user code executes on striped pool (which causes issues) > > > > The solution: don't execute user code on striped pool > > > > > > > > I believe this solves the problem and removes any and all > restrictions > > > > for async listeners/continuations. Am I missing something else? > > > > > > > > On Wed, Mar 17, 2021 at 10:16 AM Denis Garus > > > wrote: > > > > > > > > > Hello! > > > > > > > > > > As I understood, we don't try to solve the problem mentioned in the > > > IEP: > > > > > there is unexpected behavior, > > > > > users should carefully read the docs to know about this, and so on. > > > > > A thread that executes an incorrect listener hung in any case, > > > > > and the suggested solution is to change the pool which owns this > > > thread. > > > > > Did you consider an option to execute user-defined listeners with a > > > > > timeout? > > > > > I think that implicit using java pools to run a code that can be > hung > > > is > > > > a > > > > > questionable solution. > > > > > > > > > > вт, 16 мар. 2021 г. в 23:30, Alexey Kukushkin < > > > kukushkinale...@gmail.com > > > > >: > > > > > > > > > > > Pavel, > > > > > > > > > > > > So what you are saying is submitting a continuation to .NET > Thread > > > Pool > > > > > > already respects current SynchronizationContext and > > ConfigureAwait. > > > In > > > > > > this case the IEP looks complete (for some reason I thought that > we > > > > > should > > > > > > take care about the SynchronizationContext inside the Ignite.NET > > > > > > implementation). > > > > > > > > > > > > -- > > > > > > Best regards, > > > > > > Alexey > > > > > > > > > > > > > > > > > > > > > > > > -- > > > Best regards, > > > Alexey > > > > > >
Re: IEP-70: Async Continuation Executor
Pavel, thank you for your answer. Sorry, the previous version of IEP motivation highlighted what could be wrong with user listeners, so I thought that is the main problem. I'm wondering why you suggest using ForkJoinPool.commonPool as the default pool to run listeners? ср, 17 мар. 2021 г. в 11:51, Alexey Kukushkin : > My initial confusion was due to I did not know the > TaskCompletionSource.SetResult() internally handles > SynchronizationContext captured before the async operation. I thought we > would have to do it in Ignite. Now I know that and have no problems with > the IEP. > > ср, 17 мар. 2021 г. в 11:18, Pavel Tupitsyn : > > > Denis, > > > > > we don't try to solve the problem mentioned in the IEP > > > > The problem: user code executes on striped pool (which causes issues) > > The solution: don't execute user code on striped pool > > > > I believe this solves the problem and removes any and all restrictions > > for async listeners/continuations. Am I missing something else? > > > > On Wed, Mar 17, 2021 at 10:16 AM Denis Garus > wrote: > > > > > Hello! > > > > > > As I understood, we don't try to solve the problem mentioned in the > IEP: > > > there is unexpected behavior, > > > users should carefully read the docs to know about this, and so on. > > > A thread that executes an incorrect listener hung in any case, > > > and the suggested solution is to change the pool which owns this > thread. > > > Did you consider an option to execute user-defined listeners with a > > > timeout? > > > I think that implicit using java pools to run a code that can be hung > is > > a > > > questionable solution. > > > > > > вт, 16 мар. 2021 г. в 23:30, Alexey Kukushkin < > kukushkinale...@gmail.com > > >: > > > > > > > Pavel, > > > > > > > > So what you are saying is submitting a continuation to .NET Thread > Pool > > > > already respects current SynchronizationContext and ConfigureAwait. > In > > > > this case the IEP looks complete (for some reason I thought that we > > > should > > > > take care about the SynchronizationContext inside the Ignite.NET > > > > implementation). > > > > > > > > -- > > > > Best regards, > > > > Alexey > > > > > > > > > > > > -- > Best regards, > Alexey >
Re: IEP-70: Async Continuation Executor
Hello! As I understood, we don't try to solve the problem mentioned in the IEP: there is unexpected behavior, users should carefully read the docs to know about this, and so on. A thread that executes an incorrect listener hung in any case, and the suggested solution is to change the pool which owns this thread. Did you consider an option to execute user-defined listeners with a timeout? I think that implicit using java pools to run a code that can be hung is a questionable solution. вт, 16 мар. 2021 г. в 23:30, Alexey Kukushkin : > Pavel, > > So what you are saying is submitting a continuation to .NET Thread Pool > already respects current SynchronizationContext and ConfigureAwait. In > this case the IEP looks complete (for some reason I thought that we should > take care about the SynchronizationContext inside the Ignite.NET > implementation). > > -- > Best regards, > Alexey >
[jira] [Created] (IGNITE-14317) IgniteCache.removeAsync(key,val) fails inside an optimistic transaction
Denis Garus created IGNITE-14317:
Summary: IgniteCache.removeAsync(key,val) fails inside an
optimistic transaction
Key: IGNITE-14317
URL: https://issues.apache.org/jira/browse/IGNITE-14317
Project: Ignite
Issue Type: Bug
Affects Versions: 2.9.1
Reporter: Denis Garus
[reproducer|https://github.com/apache/ignite/pull/8841/files]
IgniteCache.removeAsync(key,val) fails inside an optimistic tx with the
exception:
{code:java}
[17:39:28] (err) Failed to notify listener:
o.a.i.i.processors.cache.distributed.near.GridNearTxLocal$6...@19c520dbjava.lang.AssertionError[17:39:28]
(err) Failed to notify listener:
o.a.i.i.processors.cache.distributed.near.GridNearTxLocal$6...@19c520dbjava.lang.AssertionError
at
org.apache.ignite.internal.processors.cache.distributed.near.GridNearTxLocal$17.apply(GridNearTxLocal.java:2955)
at
org.apache.ignite.internal.processors.cache.distributed.near.GridNearTxLocal$17.apply(GridNearTxLocal.java:2937)
at
org.apache.ignite.internal.processors.cache.distributed.near.GridNearTxLocal.processLoaded(GridNearTxLocal.java:3475)
at
org.apache.ignite.internal.processors.cache.distributed.near.GridNearTxLocal$21.apply(GridNearTxLocal.java:3217)
at
org.apache.ignite.internal.processors.cache.distributed.near.GridNearTxLocal$21.apply(GridNearTxLocal.java:3212)
at
org.apache.ignite.internal.util.future.GridFutureChainListener.applyCallback(GridFutureChainListener.java:78)
at
org.apache.ignite.internal.util.future.GridFutureChainListener.apply(GridFutureChainListener.java:70)
at
org.apache.ignite.internal.util.future.GridFutureChainListener.apply(GridFutureChainListener.java:30)
at
org.apache.ignite.internal.util.future.GridFutureAdapter.notifyListener(GridFutureAdapter.java:399)
at
org.apache.ignite.internal.util.future.GridFutureAdapter.unblock(GridFutureAdapter.java:347)
at
org.apache.ignite.internal.util.future.GridFutureAdapter.unblockAll(GridFutureAdapter.java:335)
at
org.apache.ignite.internal.util.future.GridFutureAdapter.onDone(GridFutureAdapter.java:511)
at
org.apache.ignite.internal.processors.cache.GridCacheFutureAdapter.onDone(GridCacheFutureAdapter.java:55)
at
org.apache.ignite.internal.util.future.GridFutureAdapter.onDone(GridFutureAdapter.java:490)
at
org.apache.ignite.internal.processors.cache.distributed.dht.GridPartitionedSingleGetFuture.onDone(GridPartitionedSingleGetFuture.java:935)
at
org.apache.ignite.internal.util.future.GridFutureAdapter.onDone(GridFutureAdapter.java:467)
at
org.apache.ignite.internal.processors.cache.distributed.dht.GridPartitionedSingleGetFuture.setSkipValueResult(GridPartitionedSingleGetFuture.java:759)
at
org.apache.ignite.internal.processors.cache.distributed.dht.GridPartitionedSingleGetFuture.onResult(GridPartitionedSingleGetFuture.java:636)
at
org.apache.ignite.internal.processors.cache.distributed.dht.GridDhtCacheAdapter.processNearSingleGetResponse(GridDhtCacheAdapter.java:368)
at
org.apache.ignite.internal.processors.cache.distributed.dht.colocated.GridDhtColocatedCache.access$100(GridDhtColocatedCache.java:88)
at
org.apache.ignite.internal.processors.cache.distributed.dht.colocated.GridDhtColocatedCache$2.apply(GridDhtColocatedCache.java:133)
at
org.apache.ignite.internal.processors.cache.distributed.dht.colocated.GridDhtColocatedCache$2.apply(GridDhtColocatedCache.java:131)
at
org.apache.ignite.internal.processors.cache.GridCacheIoManager.processMessage(GridCacheIoManager.java:1143)
at
org.apache.ignite.internal.processors.cache.GridCacheIoManager.onMessage0(GridCacheIoManager.java:592)
at
org.apache.ignite.internal.processors.cache.GridCacheIoManager.handleMessage(GridCacheIoManager.java:393)
at
org.apache.ignite.internal.processors.cache.GridCacheIoManager.handleMessage(GridCacheIoManager.java:319)
at
org.apache.ignite.internal.processors.cache.GridCacheIoManager$1.onMessage(GridCacheIoManager.java:309)
at
org.apache.ignite.internal.managers.communication.GridIoManager.invokeListener(GridIoManager.java:1908)
at
org.apache.ignite.internal.managers.communication.GridIoManager.processRegularMessage0(GridIoManager.java:1529)
at
org.apache.ignite.internal.managers.communication.GridIoManager$9.execute(GridIoManager.java:1422)
at
org.apache.ignite.internal.managers.communication.TraceRunnable.run(TraceRunnable.java:55)
at
org.apache.ignite.internal.util.StripedExecutor$Stripe.body(StripedExecutor.java:569)
at org.apache.ignite.internal.util.worker.GridWorker.run(GridWorker.java:120)
at java.base/java.lang.Thread.run(Thread.java:834){code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Created] (IGNITE-14179) The GridDhtTxFinishFuture has the redundant interface declaration 'GridCacheFuture'
Denis Garus created IGNITE-14179: Summary: The GridDhtTxFinishFuture has the redundant interface declaration 'GridCacheFuture' Key: IGNITE-14179 URL: https://issues.apache.org/jira/browse/IGNITE-14179 Project: Ignite Issue Type: Bug Reporter: Denis Garus Remove redundant interface declaration 'GridCacheFuture' -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (IGNITE-14070) Protecting a snapshot from from unauthorized changes
Denis Garus created IGNITE-14070: Summary: Protecting a snapshot from from unauthorized changes Key: IGNITE-14070 URL: https://issues.apache.org/jira/browse/IGNITE-14070 Project: Ignite Issue Type: New Feature Reporter: Denis Garus Assignee: Denis Garus We have to allow Ignite users to check the integrity of snapshot files before restoring them. This opportunity can be done through the Ignite plugin mechanism. -- This message was sent by Atlassian Jira (v8.3.4#803005)
Re: Issue with custom security plugin and thin clients
Hello! I afraid these changes will not be included in the 2.10 version. сб, 19 дек. 2020 г. в 06:03, Vishwas Bm : > Hi Denis, > > Thanks for the feedback. > > I had also put a comment in one of your PR regarding iep-41. > https://github.com/apache/ignite/pull/8038#issuecomment-742230009 > > > It will be great if you can provide input on this. > > > Regards, > Vishwas > > On Fri, 18 Dec, 2020, 21:39 Denis Garus, wrote: > > > Hi! > > I don't understand why you do something related to thin clients inside > > onDisconnected method? > > The rest looks good to me. > > > > ср, 9 дек. 2020 г. в 17:00, vbm : > > > > > Hi Denis, > > > > > > Any thoughts on the approach mentioned above ? > > > > > > > > > Regards, > > > Vishwas > > > > > > > > > > > > -- > > > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/ > > > > > >
Re: Issue with custom security plugin and thin clients
Hi! I don't understand why you do something related to thin clients inside onDisconnected method? The rest looks good to me. ср, 9 дек. 2020 г. в 17:00, vbm : > Hi Denis, > > Any thoughts on the approach mentioned above ? > > > Regards, > Vishwas > > > > -- > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/ >
Re: Issue with custom security plugin and thin clients
Hi! Node attributes can't be used to spread a thin client's security context. For this purpose, you can use a cache of Ignite, a third-party database, or other tools appropriate to your case. сб, 28 нояб. 2020 г. в 06:16, Vishwas Bm : > Hi Denis, > > > Thanks for the reply. > Yes I was looking for a way to spread the security context to all cluster > nodes when a thin client(sqlline) gets authenticated. > I tried to see if I can use node attributes or user attributes to pass the > information to other nodes. When a cluster of ignite server is already > formed, this will not help as attributes will not be available on remote > nodes. > > The node attributes cannot be changed at run time and the attributes will > be available to remote nodes only when they join the cluster. > > So I wanted to know, if there is any other way to do this ? > I checked your poc PR for reference, > https://github.com/apache/ignite/pull/7375 > > In thin client case authenticate node will not be called but authenticate > method is getting called. > > > Regards, > Vishwas > > > On Fri, 27 Nov, 2020, 14:29 Denis Garus, wrote: > > > Hello! > > > > > > If I understood your problem correctly, you need to make a thin client's > > security context allowed on a remote node. > > > > When a security plugin does authenticate a thin client, it should spread > > the thin client's security context on the cluster. > > > > How a security context will be transmitted to a remote node is up to the > > plugin's developers. > > > > Also, you have to implement the > GridSecurityProcessor.securityContext(UUID > > subjId) method, > > > > the way this method is used in Ignite can see in the task description > [1]. > > > > > > > > > >1. https://issues.apache.org/jira/browse/IGNITE-12759 > > > > > > чт, 26 нояб. 2020 г. в 10:01, Vishwas Bm : > > > > > Hi, > > > > > > I was facing an issue with a custom security plugin and thin remote > > client. > > > I am using Ignite 2.9.0 version and I am hitting below issue > > > > > > > > > https://cwiki.apache.org/confluence/display/IGNITE/IEP-41%3A+Security+Context+of+thin+client+on+remote+nodes > > > > > > > > > I had asked the question in the user listing but unfortunately I did > not > > > get any reply. > > > So I am posting this question here: > > > > > > > > > > > > http://apache-ignite-users.70518.x6.nabble.com/Query-on-implementing-GridSecurityProcessor-td34672.html > > > > > > > > > *Thanks & Regards,* > > > > > > *Vishwas * > > > > > >
Re: Issue with custom security plugin and thin clients
Hello! If I understood your problem correctly, you need to make a thin client's security context allowed on a remote node. When a security plugin does authenticate a thin client, it should spread the thin client's security context on the cluster. How a security context will be transmitted to a remote node is up to the plugin's developers. Also, you have to implement the GridSecurityProcessor.securityContext(UUID subjId) method, the way this method is used in Ignite can see in the task description [1]. 1. https://issues.apache.org/jira/browse/IGNITE-12759 чт, 26 нояб. 2020 г. в 10:01, Vishwas Bm : > Hi, > > I was facing an issue with a custom security plugin and thin remote client. > I am using Ignite 2.9.0 version and I am hitting below issue > > https://cwiki.apache.org/confluence/display/IGNITE/IEP-41%3A+Security+Context+of+thin+client+on+remote+nodes > > > I had asked the question in the user listing but unfortunately I did not > get any reply. > So I am posting this question here: > > > http://apache-ignite-users.70518.x6.nabble.com/Query-on-implementing-GridSecurityProcessor-td34672.html > > > *Thanks & Regards,* > > *Vishwas * >
[jira] [Created] (IGNITE-13652) Wrong GitHub link for Apache Ignite With Spring Data/Example
Denis Garus created IGNITE-13652: Summary: Wrong GitHub link for Apache Ignite With Spring Data/Example Key: IGNITE-13652 URL: https://issues.apache.org/jira/browse/IGNITE-13652 Project: Ignite Issue Type: Bug Components: documentation Affects Versions: 2.9 Reporter: Denis Garus Wrong GihHub link in https://ignite.apache.org/docs/latest/extensions-and-integrations/spring/spring-data#example -- This message was sent by Atlassian Jira (v8.3.4#803005)
Re: [Micronaut] pubsub support
Hello, Saikat!
I know that IgniteMessagin is used in the production environment and
I don't see any bugs in Jira with this feature.
I wrote some tests for IgniteMessaging in the security suite, and only one
confusing thing was noticed [1].
Why did we decide to deprecate IgniyeMessaging API in Ignite 3.0?
1.
http://apache-ignite-developers.2346864.n4.nabble.com/Inconsistent-behavior-of-IgniteMessaging-td47481.html
вс, 1 нояб. 2020 г. в 04:43, Michael Pollind :
> I'm looking at this data stream api and I'm having a hard time working out
> how this scheme fits into the pub sub model that micronaut uses. Here is
> what I've kind of come up with and wonder if this would work.
>
> @PubSubClientpublic interface MultipleMessage {
>
> @Topic("animals")
> public void send(@key() String key, Animal o);
>
> @Topic("my_topic")
> public void send2(@key() Long key, Object o);
> }
>
>
> @PubSubListener
>
> public class SimpleSub {
>
> @Subscription("animals")
>
> public class Listener(@key() String key, Animal o) {
>
> }
>
> }
>
>
> On Tue, Oct 27, 2020 at 7:15 PM Saikat Maitra
> wrote:
>
> > Hi Michael, Denis
> >
> > I did an initial review of the PR and wanted to share my thoughts. I
> > observed that currently the implementation is using Ignite messaging apis
> > and I am thinking if we should change the implementation to use Ignite
> data
> > streaming apis. The reason for change is to ensure that it will be better
> > aligned to Ignite 3.0 release as we are planning to deprecate Ignite
> > Messaging apis in Ignite 3.0 [1]
> >
> > My recommendation to use Ignite data streaming apis is also due to the
> > fact that we have multiple integration like Google Pub/Sub[2] and Kafka
> > Streamer[3] that uses Data streaming apis for integration and it will
> keep
> > micronaut implementation consistent with other extensions.
> >
> > Here is an example[4] of how Kafka Streamer can be used to stream data
> > into Ignite nodes.
> >
> >
> > [1] https://cwiki.apache.org/confluence/display/IGNITE/Apache+Ignite+3.0
> > [2]
> >
> https://github.com/apache/ignite-extensions/blob/master/modules/pub-sub-ext/src/main/java/org/apache/ignite/stream/pubsub/PubSubStreamer.java
> > [3]
> >
> https://github.com/apache/ignite-extensions/blob/master/modules/kafka-ext/src/main/java/org/apache/ignite/stream/kafka/KafkaStreamer.java
> > [4]
> >
> https://github.com/samaitra/streamersk-extensions/blob/main/src/main/kotlin/com/example/streamerskextensions/kafka/KafkaStreamer.kt
> >
> > Please let me know your feedback.
> >
> > Regards,
> > Saikat
> >
> > On Tue, Oct 27, 2020 at 12:48 PM Denis Magda wrote:
> >
> >> Michael, glad to see you back!
> >>
> >> Could you please explain in a few words what this integration does and
> >> what
> >> it enables for Ignite and Micronaout? So that we are as a community are
> on
> >> the same page.
> >>
> >> -
> >> Denis
> >>
> >>
> >> On Sun, Oct 25, 2020 at 3:05 PM Michael Pollind
> >> wrote:
> >>
> >> > I've started to work on adding some basic pubsub support for
> >> > Micronaut-ignite. I only have a draft PR in progress. There are a
> couple
> >> > modules that already implement pub-sub so those modules would be a
> good
> >> > start for reference:
> >> > https://github.com/micronaut-projects/micronaut-gcp
> >> > https://github.com/micronaut-projects/micronaut-kafka
> >> > https://github.com/micronaut-projects/micronaut-mqtt
> >> >
> >> > https://github.com/micronaut-projects/micronaut-ignite/pull/62
> >> >
> >> >
> >> >
> >>
> >
>
[jira] [Created] (IGNITE-13312) Methods of interface Binarylizable should not get accessing to host resources.
Denis Garus created IGNITE-13312: Summary: Methods of interface Binarylizable should not get accessing to host resources. Key: IGNITE-13312 URL: https://issues.apache.org/jira/browse/IGNITE-13312 Project: Ignite Issue Type: Task Components: security Reporter: Denis Garus Methods of interface Binarylizable should not get accessing to host resources on a remote nodes. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (IGNITE-13301) IgniteScheduler has to run inside the Ignite Sandbox.
Denis Garus created IGNITE-13301:
Summary: IgniteScheduler has to run inside the Ignite Sandbox.
Key: IGNITE-13301
URL: https://issues.apache.org/jira/browse/IGNITE-13301
Project: Ignite
Issue Type: Task
Components: security
Reporter: Denis Garus
Assignee: Denis Garus
IgniteScheduler has to run inside the Ignite Sandbox on a remote node.
For example:
{code:java}
Ignition.localIgnite().compute().run(() -> {
IgniteScheduler scheduler = Ignition.localIgnite().scheduler();
scheduler.runLocal(AbstractSandboxTest::controlAction).get();
}
{code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Created] (IGNITE-13261) Using transactions or continuous queries inside the ignite sandbox can throw an AccessControlException
Denis Garus created IGNITE-13261: Summary: Using transactions or continuous queries inside the ignite sandbox can throw an AccessControlException Key: IGNITE-13261 URL: https://issues.apache.org/jira/browse/IGNITE-13261 Project: Ignite Issue Type: Bug Components: security Reporter: Denis Garus Assignee: Denis Garus Any subject should be able to use transactions or continuous queries inside the ignite sandbox without additional permissions. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (IGNITE-13228) Remote filter of IgniteEvents has to run with appropriate SecurityContext.
Denis Garus created IGNITE-13228: Summary: Remote filter of IgniteEvents has to run with appropriate SecurityContext. Key: IGNITE-13228 URL: https://issues.apache.org/jira/browse/IGNITE-13228 Project: Ignite Issue Type: Improvement Components: security Affects Versions: 2.8.1 Reporter: Denis Garus Assignee: Denis Garus Fix For: 2.9 The remote filter of IgniteEvents has to run on a remote node with the SecurityContext of the initiator node. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (IGNITE-13113) CacheEvent#subjectId for cache events with types EventType#EVTS_CACHE
Denis Garus created IGNITE-13113: Summary: CacheEvent#subjectId for cache events with types EventType#EVTS_CACHE Key: IGNITE-13113 URL: https://issues.apache.org/jira/browse/IGNITE-13113 Project: Ignite Issue Type: Bug Components: cache, security Affects Versions: 2.8.1 Reporter: Denis Garus Assignee: Denis Garus For cache events with types: EVT_CACHE_ENTRY_CREATED, EVT_CACHE_ENTRY_DESTROYED, EVT_CACHE_OBJECT_PUT, EVT_CACHE_OBJECT_READ, EVT_CACHE_OBJECT_REMOVED, EVT_CACHE_OBJECT_LOCKED, EVT_CACHE_OBJECT_UNLOCKED, EVT_CACHE_OBJECT_EXPIRED field CacheEvent#subjectId should be subject id associated with SecuritySubject that trigged the cache event. Currently, CacheEvent#subjectId for these event types is null or equals subject id associated with the node that sends a change request. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (IGNITE-13112) CacheEvent#subjectId is always null for cache events with types EVT_CACHE_STARTED and EVT_CACHE_STOPPED
Denis Garus created IGNITE-13112: Summary: CacheEvent#subjectId is always null for cache events with types EVT_CACHE_STARTED and EVT_CACHE_STOPPED Key: IGNITE-13112 URL: https://issues.apache.org/jira/browse/IGNITE-13112 Project: Ignite Issue Type: Bug Components: cache, security Reporter: Denis Garus CacheEvents with types EVT_CACHE_STARTED and EVT_CACHE_STARTED have fieldCacheEvent#subjectId always null. CacheEvent#subjectId should be subject id associated with SecuritySubject that trigged the cache event. -- This message was sent by Atlassian Jira (v8.3.4#803005)
Inconsistent behavior of IgniteMessaging
Hello, Igniters!
IgniteMessaging has inconsistent behavior when a remote listener throws an
exception.
A remote listener throws an exception, if this listener registered on a
node that sends a message,
the sender gets this exception. But if the sender node and the node with
the remote listener aren't the same,
no one will get this exception.
I think the right behavior is to write an exception into a log and ignore
the exception.
WDYT?
The reproducer:
```
public class InconsistentBehaviorOfIgniteMessagingReproducer extends
GridCommonAbstractTest {
@Test
public void test() throws Exception {
startGrids(3);
String topic = "test_topic";
grid(0).message(grid(0).cluster().forNodeId(grid(2).localNode().id()))
.remoteListen(topic, (uuid, msg) -> {
throw new RuntimeException("Ops!");
});
try {
// This line doesn't throw an exception.
grid(1).message().send(topic, "Hello!");
}
catch (Exception e) {
fail("Shouldn't be any exception");
}
// This line throws java.lang.RuntimeException: Ops!
grid(2).message().send(topic, "Hello!");
}
}
```
[jira] [Created] (IGNITE-13010) A local listener for cache events with type EVT_CACHE_STOPPED does not get a cache event from a remote node.
Denis Garus created IGNITE-13010:
Summary: A local listener for cache events with type
EVT_CACHE_STOPPED does not get a cache event from a remote node.
Key: IGNITE-13010
URL: https://issues.apache.org/jira/browse/IGNITE-13010
Project: Ignite
Issue Type: Bug
Affects Versions: 2.8
Reporter: Denis Garus
A local listener for cache events with type EVT_CACHE_STOPPED does not get a
cache event from a remote node.
That occurs due to NPE on a remote node:
{code:java}
[2020-05-14
12:07:25,623][ERROR][sys-#206%security.NpeGridEventConsumeHandlerReproducer2%][GridEventConsumeHandler]
Failed to send event notification to node:
55671ec1-dad9-452b-8ab2-4b7916c0[2020-05-14
12:07:25,623][ERROR][sys-#206%security.NpeGridEventConsumeHandlerReproducer2%][GridEventConsumeHandler]
Failed to send event notification to node:
55671ec1-dad9-452b-8ab2-4b7916c0java.lang.NullPointerException at
org.apache.ignite.internal.GridEventConsumeHandler$2$1.run(GridEventConsumeHandler.java:238)
at
java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
at
java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
at java.base/java.lang.Thread.run(Thread.java:834)
{code}
The reproducer:
{code:java}
public class NpeGridEventConsumeHandlerReproducer extends
GridCommonAbstractTest {
private static AtomicInteger rmtCounter = new AtomicInteger();
private static AtomicInteger locCounter = new AtomicInteger();
@Override protected IgniteConfiguration getConfiguration(String
igniteInstanceName) throws Exception {
return
super.getConfiguration(igniteInstanceName).setIncludeEventTypes(EVT_CACHE_STOPPED);
}
@Test
public void test() throws Exception {
startGrids(3);
grid(1).createCache(new CacheConfiguration<>("test_cache"));
grid(0).events().remoteListen((uuid, evt) ->{
locCounter.incrementAndGet();
return true;
}, evt->{
rmtCounter.incrementAndGet();
return true;
}, EVT_CACHE_STOPPED);
grid(1).destroyCache("test_cache");
TimeUnit.SECONDS.sleep(10);
assertEquals(rmtCounter.get(), locCounter.get());
}
}
{code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Created] (IGNITE-12996) Remote listener of IgniteEvents has to run inside the Ignite Sandbox.
Denis Garus created IGNITE-12996: Summary: Remote listener of IgniteEvents has to run inside the Ignite Sandbox. Key: IGNITE-12996 URL: https://issues.apache.org/jira/browse/IGNITE-12996 Project: Ignite Issue Type: Task Components: security Reporter: Denis Garus Remote listener of IgniteEvents has to run on a remote node inside the Ignite Sandbox if it is turned on. -- This message was sent by Atlassian Jira (v8.3.4#803005)
Re: [ANNOUNCE] New PMC member: Maxim Muzafarov
Maxim, Congrats! Great job! чт, 7 мая 2020 г. в 15:02, Nikita Amelchev : > Maxim, congrats! > > чт, 7 мая 2020 г. в 14:55, Nikolay Izhikov : > > > > Congrats. > > > > > 7 мая 2020 г., в 14:54, Ivan Pavlukhin > написал(а): > > > > > > Maxim, > > > > > > My congratulations! Well deserved! > > > > > > P.S. We should mention snapshots for persistent caches in the > > > achievement list. Great job! > > > > > > Best regards, > > > Ivan Pavlukhin > > > > > > чт, 7 мая 2020 г. в 14:47, Dmitriy Pavlov : > > >> > > >> The Project Management Committee (PMC) for Apache Ignite > > >> > > >> has invited Maxim Muzafarov to become new PMC member and we are > pleased to > > >> announce that he has accepted. > > >> > > >> > > >> > > >> Maxim is active dev list participant, speaker at meetups, and > contributes > > >> to additional checks of the product using travis and started new file > > >> rebalance. Maxim did a fantastic job to make release 2.8 possible > > >> > > >> > > >> > > >> Being a PMC member enables assistance with the management > > >> > > >> and to guide the direction of the project. > > >> > > >> Maxim, congrats with new role and keep the pace ! > > >> > > >> > > >> > > >> Best Regards, > > >> > > >> Dmitriy Pavlov > > >> > > >> on behalf of Apache Ignite PMC > > > > > -- > Best wishes, > Amelchev Nikita >
Re: Apache Ignite 2.8.1 RELEASE [Time, Scope, Manager]
gt;>>>>>> [2] > > >>>>>>> > > >> > > > https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8==testDetails=-3485591522651012009=TEST_STATUS_DESC_IgniteTests24Java8=__all_branches__=50 > > >>>>>>>> > > >>>>>>>>> 21 апр. 2020 г., в 17:49, Ivan Bessonov > > > >>>>>>> написал(а): > > >>>>>>>>> Sure, here's PR with 3 cherry-picked commits that I mentioned: > > >>>>>>>>> https://github.com/apache/ignite/pull/7708 > > >>>>>>>>> > > >>>>>>>>> вт, 21 апр. 2020 г. в 17:17, Nikolay Izhikov < > > nizhi...@apache.org > > >>> : > > >>>>>>>>> > > >>>>>>>>>> OK then, let’s include it in the 2.8.1 > > >>>>>>>>>> > > >>>>>>>>>> Ivan, can you, please, prepare PR in the ignite-2.8.1 branch > > that > > >>>>>>> contain > > >>>>>>>>>> cherry-pick for all required commits? > > >>>>>>>>>> > > >>>>>>>>>>> 21 апр. 2020 г., в 17:01, Andrey Gura > > >> написал(а): > > >>>>>>>>>>> > > >>>>>>>>>>> Hi > > >>>>>>>>>>> > > >>>>>>>>>>>> IGNITE-12735 - Metric exporter implementation could lead to > > >>>>>>>>>> NullPointerException from gauge which invoke communication > > >>>>>>>>>>>> IGNITE-12568 - MessageFactory implementations refactoring > > >>>>>>>>>>>> Personally, I’m against any refactoring improvements in bug > > fix > > >>>>>>> release. > > >>>>>>>>>>>> So, I propose to exclude IGNITE-12756 from 2.8.1 > > >>>>>>>>>>>> Andrey, what do you think as a committer of this > improvements? > > >>>>>>>>>>> Mainly IGNITE-12756 brings some improvements related with TCP > > >>>>>>>>>>> communication metrics (performance, memory footprint, > > >>>>>>> IgniteSpiContext > > >>>>>>>>>>> improved in order to provide ability to implement metrics > > related > > >>>>>>>>>>> SPI's without using internal API's, code improvements) > > >>>>>>>>>>> > > >>>>>>>>>>> But! It also fixes potential NPE's which can be thrown on > node > > >> start. > > >>>>>>>>>>> So it would be great to include this fix to 2.8.1 release. > > >>>>>>>>>>> > > >>>>>>>>>>> On Tue, Apr 21, 2020 at 11:12 AM Nikolay Izhikov < > > >>>>>>> nizhi...@apache.org> > > >>>>>>>>>> wrote: > > >>>>>>>>>>>>> I've cherry-picked IGNITE-12734 to 2.8.1 branch. > > >>>>>>>>>>>> > > >>>>>>>>>>>> Thanks! > > >>>>>>>>>>>> > > >>>>>>>>>>>>> considering commit "683f22e64f IGNITE-12756 > TcpCommunication > > >> SPI > > >>>>>>>>>> metrics > > >>>>>>>>>>>>>> improvement" - it depends > > >>>>>>>>>>>>>> on https://issues.apache.org/jira/browse/IGNITE-12735 and > > >>>>>>>>>>>>>> https://issues.apache.org/jira/browse/IGNITE-12568, > > >>>>>>>>>>>>>> both were targeted to 2.9, but this has to be changed > > >> probably. > > >>>>>>>>>>>> IGNITE-12735 - Metric exporter implementation could lead to > > >>>>>>>>>> NullPointerException from gauge which invoke communication > > >>>>>>>>>>>> IGNITE-12568 - MessageFactory implementations refactoring > > >>>>>>>>>>>> > > >>>>>>>>>>>&
[jira] [Created] (IGNITE-12983) Logging exceptions inside IgniteSecurityProcessor#withContext(java.util.UUID)
Denis Garus created IGNITE-12983: Summary: Logging exceptions inside IgniteSecurityProcessor#withContext(java.util.UUID) Key: IGNITE-12983 URL: https://issues.apache.org/jira/browse/IGNITE-12983 Project: Ignite Issue Type: Improvement Reporter: Denis Garus Assignee: Denis Garus Fix For: 2.9 We should write down to log all exception inside IgniteSecurityProcessor#withContext(java.util.UUID). -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (IGNITE-12904) ШптшеуЫусгкшен
Denis Garus created IGNITE-12904: Summary: ШптшеуЫусгкшен Key: IGNITE-12904 URL: https://issues.apache.org/jira/browse/IGNITE-12904 Project: Ignite Issue Type: Improvement Reporter: Denis Garus -- This message was sent by Atlassian Jira (v8.3.4#803005)
Re: [DISCUSSION] Pull-request checks on GitHub
Hello! I have seen projects with Travis-ci they look cool. I think Travis-ci is a good solution. вт, 14 апр. 2020 г. в 10:00, Andrey Mashenkov : > Maxim, > > Good idea. I'd add a license check as well. > > On Tue, Apr 14, 2020 at 2:14 AM Maxim Muzafarov wrote: > > > Igniters, > > > > It's really `must-have` feature for me to enable Apache Ignite > > pull-request status checks on GitHub. Currently we don't have any of > > them. The most obvious check for each pull-request is: > > - build the source code and check code-style violations; > > > > This will give us some advantages. For instance: > > 1. Each PR even a very simple (not require tests execution) will be > > checked by checkstyle and for compile errors. > > 2. Developers can be get notified earlier if checkstyle has been > > violated in their PRs. > > > > To achieve this we can do the following: > > 1. Configure our TeamCity integration with the Ignite GitHub > > repository and PR build. It seems it is possible [2]. > > 2. Use Travis-ci which is free for open-source projects and also has > > an integration with GitHub checks [1]. > > > > > > What do you think? > > What options will be the best for us? > > > > [1] > > > https://blog.travis-ci.com/2018-05-07-announcing-support-for-github-checks-api-on-travis-ci-com > > [2] > > > https://himynameistim.com/2018/01/16/adding-build-statuses-to-pull-requests-with-teamcity-and-github/ > > > > > -- > Best regards, > Andrey V. Mashenkov >
Re: Get rid of @Nullable and @NotNull
Hi! I'm not sure that @Nullable can really fix the NPE problem. Currently, we have @Nullable annotation and NPE simultaneously. The best way to avoid NPE is by using a null object pattern. I agree we shouldn't rely on @Nullable. пт, 27 мар. 2020 г. в 12:58, Sergey Antonov : > I disagree. > > Intellij idea IDE has a static code analysis, which uses that > annotation too. IDE highlights possible problems. It helps to make our code > more stable and bugless. > > пт, 27 мар. 2020 г. в 12:06, Pavel Tupitsyn : > > > I disagree, it would be a step back. > > > > > What's the reason for using it? > > Null was a billion dollar mistake [1]. > > NullPointerExceptions happen quite a lot in Ignite, and annotations > provide > > some clues to avoid those. > > > > [1] https://en.wikipedia.org/wiki/Tony_Hoare#Apologies_and_retractions > > > > On Fri, Mar 27, 2020 at 10:58 AM Anton Vinogradov wrote: > > > > > Folks, > > > > > > Found we still use @Nullable annotation. > > > > > > What's the reason for using it? > > > Everything is Object and Nullable :) > > > > > > How about get rid of @Nullable usages and restrict its usage by > > checkstyle > > > plugin? > > > > > > BTW, We already "do not use @NotNull annotation" (с) Coding Guidelines > > [1] > > > which may have some cense in contrast to @Nullable. > > > But I see a lot of usages at code. > > > > > > How about to get rid of @NotNull too and to add such check to > checkstyle > > > plugin too? > > > > > > [1] > > > > > > > > > https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-@Annotations > > > > > > > > -- > BR, Sergey Antonov >
Re: Security Subject of thin client on remote nodes
Hi, guys! I agree that we should rework the security API, but it can take a long time. And currently, our users have certain impediments that are blockers for their job. I think we have to fix bugs that IEP-41 [1] contains as soon as possible to support our users. From my point of view, IEP-41 is the best place to track bug fixing. 1. https://cwiki.apache.org/confluence/display/IGNITE/IEP-41%3A+Security+Context+of+thin+client+on+remote+nodes вт, 24 мар. 2020 г. в 12:26, Ivan Rakov : > Alexey, > > That can be another version of our plan. If everyone agrees that > SecurityContext and SecuritySubject should be merged, such fix of thin > clients' issue will bring us closer to the final solution. > Denis, what do you think? > > On Tue, Mar 24, 2020 at 10:38 AM Alexei Scherbakov < > alexey.scherbak...@gmail.com> wrote: > > > Why can't we start gradually changing security API right now ? > > I see no point in delaying with. > > All changes will go to next 2.9 release anyway. > > > > My proposal: > > 1. Get rid of security context. Doing this will bring security API to > more > > or less consistent state. > > 2. Remove IEP-41 because it's no longer needed because of change [1] > > 3. Propose an IEP to make security API avoid using internals. > > > > > > > > пн, 23 мар. 2020 г. в 19:53, Denis Garus : > > > > > Hello, Alexei, Ivan! > > > > > > >> Seems like security API is indeed a bit over-engineered > > > > > > Nobody has doubt we should do a reworking of GridSecurityProcessor. > > > But this point is outside of scope thin client's problem that we are > > > solving. > > > I think we can create new IEP that will accumulate all ideas of > Ignite's > > > security improvements. > > > > > > >> Presence of the separate #securityContext(UUID) highlights that user > > > indeed should care > > > >> about propagation of thin clients' contexts between the cluster > nodes. > > > > > > I agree with Ivan. I've implemented both variants, > > > and I like one with #securityContext(UUID) more. > > > > > > Could you please take a look at PR [1] for the issue [2]? > > > > > > 1. https://github.com/apache/ignite/pull/7523 > > > 2. https://issues.apache.org/jira/browse/IGNITE-12759 > > > > > > пн, 23 мар. 2020 г. в 11:45, Ivan Rakov : > > > > > > > Alex, Denis, > > > > > > > > Seems like security API is indeed a bit over-engineered. > > > > > > > > Let's get rid of SecurityContext and use SecuritySubject instead. > > > > > SecurityContext is just a POJO wrapper over > > > > > SecuritySubject's > > > > > org.apache.ignite.plugin.security.SecuritySubject#permissions. > > > > > It's functionality can be easily moved to SecuritySubject. > > > > > > > > I totally agree. Both subject and context are implemented by plugin > > > > provider, and I don't see any reason to keep both abstractions, > > > especially > > > > if we are going to get rid of transferring subject in node attributes > > > > (argument that subject is more lightweight won't work anymore). > > > > > > > > Also, there's kind of mess in node authentication logic. There are at > > > least > > > > two components responsible for it: DiscoverySpiNodeAuthenticator > (which > > > is > > > > forcibly set by GridDiscoveryManager, but in fact public) and > > > > GridSecurityProcessor (which performs actual node auth logic, but > > > private). > > > > I also don't understand why we need both > > > > #authenticate(AuthenticationContext) and > #authenticateNode(ClusterNode, > > > > SecurityCredentials) methods while it's possible to set explicit > > > > SecuritySubjectType.REMOTE_NODE in AuthenticationContext (this is > > > arguable; > > > > perhaps there are strong reasons). > > > > > > > > Finally, areas of responsibility between IgniteSecurity and > > > > GridSecurityProcessor are kind of mixed. As far as I understand, the > > > first > > > > is responsible for Ignite-internal management of security logic > > (keeping > > > > thread-local context, caching security contexts, etc; we don't expect > > > > IgniteSecurity to be replaced by plugin provider) and the latter is > > > > responsible for user-custom authentication / authorization logic. To > be >
Re: Security Subject of thin client on remote nodes
Hello, Alexei, Ivan! >> Seems like security API is indeed a bit over-engineered Nobody has doubt we should do a reworking of GridSecurityProcessor. But this point is outside of scope thin client's problem that we are solving. I think we can create new IEP that will accumulate all ideas of Ignite's security improvements. >> Presence of the separate #securityContext(UUID) highlights that user indeed should care >> about propagation of thin clients' contexts between the cluster nodes. I agree with Ivan. I've implemented both variants, and I like one with #securityContext(UUID) more. Could you please take a look at PR [1] for the issue [2]? 1. https://github.com/apache/ignite/pull/7523 2. https://issues.apache.org/jira/browse/IGNITE-12759 пн, 23 мар. 2020 г. в 11:45, Ivan Rakov : > Alex, Denis, > > Seems like security API is indeed a bit over-engineered. > > Let's get rid of SecurityContext and use SecuritySubject instead. > > SecurityContext is just a POJO wrapper over > > SecuritySubject's > > org.apache.ignite.plugin.security.SecuritySubject#permissions. > > It's functionality can be easily moved to SecuritySubject. > > I totally agree. Both subject and context are implemented by plugin > provider, and I don't see any reason to keep both abstractions, especially > if we are going to get rid of transferring subject in node attributes > (argument that subject is more lightweight won't work anymore). > > Also, there's kind of mess in node authentication logic. There are at least > two components responsible for it: DiscoverySpiNodeAuthenticator (which is > forcibly set by GridDiscoveryManager, but in fact public) and > GridSecurityProcessor (which performs actual node auth logic, but private). > I also don't understand why we need both > #authenticate(AuthenticationContext) and #authenticateNode(ClusterNode, > SecurityCredentials) methods while it's possible to set explicit > SecuritySubjectType.REMOTE_NODE in AuthenticationContext (this is arguable; > perhaps there are strong reasons). > > Finally, areas of responsibility between IgniteSecurity and > GridSecurityProcessor are kind of mixed. As far as I understand, the first > is responsible for Ignite-internal management of security logic (keeping > thread-local context, caching security contexts, etc; we don't expect > IgniteSecurity to be replaced by plugin provider) and the latter is > responsible for user-custom authentication / authorization logic. To be > honest, it took plenty of time to figure this out for me. > > From my point of view, we should make GridSecurityProcessor interface > public, rename it (it requires plenty of time to find the difference from > IgniteSecurity), make its API as simple and non-duplicating as possible and > clarify its area of responsibility (e.g. should it be responsible for > propagation of successfully authenticated subject among all nodes or not?) > to make it easy to embed custom security logic in Ignite. > > Regarding thin clients fix: implementation made by Denis suits better to > the very implicit contract that it's better to change API contracts of an > internal IgniteSecurity than of internal GridSecurityProcessor (which > actually mustn't be internal). > > > My approach doesn't require any IEPs, just minor change in code and to > > > > > org.apache.ignite.internal.processors.security.IgniteSecurity#authenticate(AuthenticationContext) > > contract. > > Looks like a misuse of #authenticate method to me. It should perform > initial authentication based on credentials (this may include queries to > external authentication subsystem, e.g. LDAP). User may want to don't > authenticate thin client on every node (this will increase the number of > requests to auth subsystem unless user implicitly implements propagation of > thin clients' contexts between nodes and make #authenticate cluster-wide > idempotent: first call should perform actual authentication, next calls > should retrieve context of already authenticated client). Presence of the > separate #securityContext(UUID) highlights that user indeed should care > about propagation of thin clients' contexts between the cluster nodes. > > -- > Ivan > > On Fri, Mar 20, 2020 at 12:22 PM Veena Mithare > wrote: > > > Hi Alexei, Denis, > > > > One of the main usecases of thin client authentication is to be able to > > audit the changes done using the thin client user. > > To enable that : > > We really need to resolve this concern as well : > > https://issues.apache.org/jira/browse/IGNITE-12781 > > > > ( Incorrect security subject id is associated with a cache_put event > > when the originator of the event is a thin client. ) > > > > Regards, > &g
Re: [REVIEW] IGNITE-12344 Remote listener of IgniteMessaging has to run with appropriate SecurityContext
Hello, Alexei! Thank you for the review! I`ve fixed your comments, take a look, please. пн, 17 февр. 2020 г. в 12:53, Nikita Amelchev : > Denis, > > I have pre-reviewed your changes, LGTM. > > Could some security maintainer take a look? > > вт, 11 февр. 2020 г. в 20:37, Denis Garus : > > > > Hi, Igniters! > > > > > > I've prepared the PR[1] for the ticket[2], could somebody do a review? > > > > > > Thanks. > > > > > > > >1. https://github.com/apache/ignite/pull/7338 > >2. https://issues.apache.org/jira/browse/IGNITE-12344 > > > > -- > Best wishes, > Amelchev Nikita >
Re: Security Subject of thin client on remote nodes
Hello, Alexei!
I agree with you if we may not care about compatibility at all,
then we can solve the problem much more straightforward way.
In your case, the method GridSecurityProcessor#authenticate will have an
implicit contract:
[ if actx.subject() != null then
returns SecurityContext
else
do authenticate ]
It looks fragile.
When we extend the GridSecurityProcessor, there isn't this problem:
we have the explicit contract and can make default implementation
that throws an unsupported operation exception to enforcing compatibility
check.
In any case, we need to change GridSecurityProcessor implementation.
But I think your proposal to try to find a security context in the node's
attributes first is right
for backward compatibility when Ignite users don't use thin clients.
Summary:
I suggest adding a new method to GridSecurityProcessor because
it has a clear contract and enforces compatibility check natural way.
вс, 15 мар. 2020 г. в 17:13, Alexei Scherbakov :
> Denis Garus,
>
> I've looked at the IEP proposed by you and currently I'm thinking it's not
> immediately required.
>
> The problem of missing SecurityContexts of thin clients can be solved much
> easily.
>
> Below is the stub of a fix, it requires correct implementation of
> method
> org.apache.ignite.internal.processors.security.IgniteSecurityProcessor#authenticatedSubject
> by GridSecurityProcessor:
>
> /** {@inheritDoc} */
> @Override public OperationSecurityContext withContext(UUID nodeId) {
> try {
> SecurityContext ctx0 = secCtxs.get(nodeId);
>
> if (ctx0 == null) {
> ClusterNode node =
> Optional.ofNullable(ctx.discovery().node(nodeId))
> .orElseGet(() ->
> ctx.discovery().historicalNode(nodeId));
>
> // This is a cluster node.
> if (node != null)
> ctx0 = nodeSecurityContext(marsh,
> U.resolveClassLoader(ctx.config()), findNode(nodeId));
> else {
> // This is already authenticated thin client.
> SecuritySubject subj = authenticatedSubject(nodeId);
>
> assert subj != null : "Subject is null " + nodeId;
>
> AuthenticationContext actx = new
> AuthenticationContext();
> actx.subject(subj);
>
> ctx0 = secPrc.authenticate(actx);
> }
> }
>
> secCtxs.putIfAbsent(nodeId, ctx0);
>
> return withContext(ctx0);
> } catch (IgniteCheckedException e) {
> throw new IgniteException(e);
> }
>
> The idea is to create a thin client SecurityContext on a node not having a
> local context using existing SecuritySubject data.
>
> Method
> org.apache.ignite.internal.processors.security.GridSecurityProcessor#authenticate
> should check for not null SecuritySubject field and just recreate
> SecurityContext using passed info (because it's already authenticated).
>
> We have all necessary information in SecuritySubject returned by
>
> org.apache.ignite.internal.processors.security.IgniteSecurityProcessor#authenticatedSubject
> by GridSecurityProcessor method.
>
> Because it is internal API, we may not care about compatibility at all,
> but nevertheless it is possible to add compatibility check in the method
> above. If a feature is not supported the operations from thin clients
> should be forbidden.
>
> You proposal has the similar problem: if GridSecurityProcessor does not
> support retriving context for thin clients, such clients will not be able
> to proceed with operation.
>
> Still, the cleanup of security API is necessary and should be done in 3.0
>
>
>
> чт, 12 мар. 2020 г. в 16:48, VeenaMithare :
>
> > HI ,
> >
> > Created this jira :
> > https://issues.apache.org/jira/browse/IGNITE-12781
> >
> > regards,
> > Veena.
> >
> >
> >
> > --
> > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
> >
>
>
> --
>
> Best regards,
> Alexei Scherbakov
>
Re: Security Subject of thin client on remote nodes
> I dont think that is true. /** * *Gets authenticated node subject.* * * @param subjId Subject ID. * @return Security subject. * @throws IgniteCheckedException If error occurred. */ public SecuritySubject authenticatedSubject(UUID subjId) throws IgniteCheckedException; > Nothing stops an implementer to return a Remote Client Security Subject. We must provide backward compatibility inside minor versions that is why we cannot expect inside the Ignite code base that an implementer returns a Remote Client Security Subject. > The primary issue is how to link the event which contains the remote node uuid ( but was originated by a remote client ) , to the originator remote client. Could you please create the issue with label iep-41? We'll fix it soon. ср, 11 мар. 2020 г. в 16:39, VeenaMithare : > >>2. Method GridSecurityProcessor#authenticatedSubject returns > authenticated *node > subject* (from JavaDoc). > > I dont think that is true. Nothing stops an implementer to return a Remote > Client Security Subject. > > The primary issue is how to link the event which contains the remote node > uuid ( but was originated by a remote client ) , to the originator remote > client. > > > > -- > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/ >
Re: Security Subject of thin client on remote nodes
1.Return type. SecurityContext is implemented by security plugin developers, and we cannot make SecurityContext from SecuritySubject. But GridSecurityProcessor#authorize method requires SecurityContext as parameter. 2. Method GridSecurityProcessor#authenticatedSubject returns authenticated *node subject* (from JavaDoc). It means we don't get a subject as a remote client using this method. GridSecurityProcessor#securityContext will not have this restriction. But I agree with you interface GridSecurityProcessor is too tricky. We could do a reworking of this interface for the 3.0 version of Ignite. ср, 11 мар. 2020 г. в 14:06, VeenaMithare : > Hi Denis, > > Thank you for working on the security issues w.r.to thin client logins . > We > plan to use the thin clients extensively in our project , it will really > help if the blockers around it are resolved, > > regards, > Veena. > > > > -- > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/ >
Re: Security Subject of thin client on remote nodes
Hi guys! I created the iep ticket [1] and started work. 1. https://issues.apache.org/jira/browse/IGNITE-12759 чт, 5 мар. 2020 г. в 12:00, Denis Garus : > Hi, guys! > > > I've prepared the IEP-41: Security Context of a thin client on remote > nodes [1]; take a look, please. > > If there aren't any questions, I could create an issue and start work. > > > Ivan, could you be an IEP sponsor? > > > Thx! > > > >1. > > https://cwiki.apache.org/confluence/display/IGNITE/IEP-41%3A+Security+Context+of+a+thin+client+on+remote+nodes > > > ср, 26 февр. 2020 г. в 12:42, Mikhail Petrov : > >> Hi, Alexei. >> >> The ticket [1] describes the general problem for all thin client >> authorizations. Its particular case is described in the mentioned in [1] >> ticket [2] on the example of a JDBC client with the reproducer attached. >> >> [1] https://issues.apache.org/jira/browse/IGNITE-12589 >> [2] https://issues.apache.org/jira/browse/IGNITE-12579 >> >> On 26.02.2020 11:47, Alexei Scherbakov wrote: >> > Denis Garus, >> > >> > It is forbidden to remove any public IGNITE attribute without proper >> > deprecation steps. >> > >> > I have read the thread and still do not clearly see the problem. The >> subject id >> > is not required to be a node id. >> > >> > The referenced in the thread ticket [1] do not provide any reproducers. >> > >> > Can you properly demonstrate a broken scenario ? >> > >> > [1] https://issues.apache.org/jira/browse/IGNITE-12589 >> > >> > пт, 21 февр. 2020 г. в 13:35, Andrey Kuznetsov : >> > >> >> Hi, guys! >> >> >> >> The change suggested by Denis looks robust to me: it covers security >> >> subject handling by all kinds of clients/nodes at once. As for >> >> ATTR_SECURITY_SUBJECT_V2 attribute, it is really better to move it to >> >> plugin implementations to support backward compatibility with peer >> nodes of >> >> older versions. Obviously, cluster with security disabled will not >> suffer >> >> from attribute removal. Ignite core should know nothing about the >> specific >> >> way of security context propagation. >> >> >> >> Denis, could you please create Jira issue for your change? >> >> >> >> чт, 20 февр. 2020 г. в 17:01, Denis Garus : >> >> >> >>>> I just transmitted security subjects for rest requests. >> >>> SecurityContext has an unlimited size so we can get significant >> overhead. >> >>> And we do not solve problems with other thin clients. >> >>> >> >>>> If you remove ATTR_SECURITY_SUBJECT_V2, it breaks compatibility >> between >> >>> old >> >>> versions and new. >> >>> >> >>> I suggest removing ATTR_SECURITY_SUBJECT_V2 from Ignite's codebase, >> but >> >> for >> >>> compatibility, it can be used by a security plugin like in PoC. >> >>> >> >>> чт, 20 февр. 2020 г. в 16:47, Maksim Stepachev < >> >> maksim.stepac...@gmail.com >> >>>> : >> >>>> Yes, I said about it at 07.19. >> >>>> >> >>>> >> >> >> http://apache-ignite-developers.2346864.n4.nabble.com/Improvements-for-new-security-approach-td42698.html#a42708 >> >>>> And in my solution, I just transmitted security subjects for rest >> >>> requests. >> >>>> If you remove ATTR_SECURITY_SUBJECT_V2, it breaks compatibility >> between >> >>> old >> >>>> versions and new. >> >>>> >> >>>> чт, 20 февр. 2020 г. в 15:56, Denis Garus : >> >>>> >> >>>>> Hi, Igniters! >> >>>>> >> >>>>> >> >>>>> At present, a security subject id is assumed to be node id. >> >>>>> >> >>>>> But when we are dealing with thin client, JDBC or REST subject id is >> >>>> random >> >>>>> UUID. In this case, we cannot get the subject information on a >> remote >> >>>> node, >> >>>>> and we get problems like these [1], [2]. >> >>>>> >> >>>>> To fix the problem, we should spread the client session to the whole >> >>>>> cluster. >> >>>>> >> >>>>> >> >>>>> I want to suggest a solution to the problem. >> >>>>> >> >>>>> >> >>>>> First, we should get subject information using >> GridSecurityProcessor. >> >>>>> >> >>>>> How GridSecurityProcessor will retrieve a subject data, it is up to >> >>>> plugin >> >>>>> developers. >> >>>>> >> >>>>> >> >>>>> Second, we should get rid of the assumption that a subject id is a >> >> node >> >>>> id >> >>>>> and remove the ATTR_SECURITY_SUBJECT_V2 attribute. >> >>>>> >> >>>>> >> >>>>> I have prepared PoC PR [3] that: >> >>>>> >> >>>>> - places the existing logic of spreading security context to >> >>>>> GridSecurityProcessor; >> >>>>> >> >>>>> - uses GridSecurityProcessor to get SecurityContext. >> >>>>> >> >>>>> >> >>>>> >> >>>>> 1. >> >>>>> >> >>>>> >> >> >> http://apache-ignite-developers.2346864.n4.nabble.com/JDBC-thin-client-incorrect-security-context-td45929.html >> >>>>> 2. https://issues.apache.org/jira/browse/IGNITE-12589 >> >>>>> 3. https://github.com/apache/ignite/pull/7375 >> >>>>> >> >> >> >> -- >> >> Best regards, >> >>Andrey Kuznetsov. >> >> >> > >> >
[jira] [Created] (IGNITE-12759) Getting a SecurityContext from GridSecurityProcessor
Denis Garus created IGNITE-12759: Summary: Getting a SecurityContext from GridSecurityProcessor Key: IGNITE-12759 URL: https://issues.apache.org/jira/browse/IGNITE-12759 Project: Ignite Issue Type: Improvement Components: security Affects Versions: 2.8 Reporter: Denis Garus Fix For: 2.8.1 1. Extend the _GridSecurityProcessor_ interface by adding _securityContext(UUID subjId)_ method and use this method to get the actual security context. 2. In the case when _GridSecurityProcessor_ cannot get a security context, we should return _the unknown security context_. All operations are forbidden with _the unknown security context_. -- This message was sent by Atlassian Jira (v8.3.4#803005)
Re: Security Subject of thin client on remote nodes
Hi, guys! I've prepared the IEP-41: Security Context of a thin client on remote nodes [1]; take a look, please. If there aren't any questions, I could create an issue and start work. Ivan, could you be an IEP sponsor? Thx! 1. https://cwiki.apache.org/confluence/display/IGNITE/IEP-41%3A+Security+Context+of+a+thin+client+on+remote+nodes ср, 26 февр. 2020 г. в 12:42, Mikhail Petrov : > Hi, Alexei. > > The ticket [1] describes the general problem for all thin client > authorizations. Its particular case is described in the mentioned in [1] > ticket [2] on the example of a JDBC client with the reproducer attached. > > [1] https://issues.apache.org/jira/browse/IGNITE-12589 > [2] https://issues.apache.org/jira/browse/IGNITE-12579 > > On 26.02.2020 11:47, Alexei Scherbakov wrote: > > Denis Garus, > > > > It is forbidden to remove any public IGNITE attribute without proper > > deprecation steps. > > > > I have read the thread and still do not clearly see the problem. The > subject id > > is not required to be a node id. > > > > The referenced in the thread ticket [1] do not provide any reproducers. > > > > Can you properly demonstrate a broken scenario ? > > > > [1] https://issues.apache.org/jira/browse/IGNITE-12589 > > > > пт, 21 февр. 2020 г. в 13:35, Andrey Kuznetsov : > > > >> Hi, guys! > >> > >> The change suggested by Denis looks robust to me: it covers security > >> subject handling by all kinds of clients/nodes at once. As for > >> ATTR_SECURITY_SUBJECT_V2 attribute, it is really better to move it to > >> plugin implementations to support backward compatibility with peer > nodes of > >> older versions. Obviously, cluster with security disabled will not > suffer > >> from attribute removal. Ignite core should know nothing about the > specific > >> way of security context propagation. > >> > >> Denis, could you please create Jira issue for your change? > >> > >> чт, 20 февр. 2020 г. в 17:01, Denis Garus : > >> > >>>> I just transmitted security subjects for rest requests. > >>> SecurityContext has an unlimited size so we can get significant > overhead. > >>> And we do not solve problems with other thin clients. > >>> > >>>> If you remove ATTR_SECURITY_SUBJECT_V2, it breaks compatibility > between > >>> old > >>> versions and new. > >>> > >>> I suggest removing ATTR_SECURITY_SUBJECT_V2 from Ignite's codebase, but > >> for > >>> compatibility, it can be used by a security plugin like in PoC. > >>> > >>> чт, 20 февр. 2020 г. в 16:47, Maksim Stepachev < > >> maksim.stepac...@gmail.com > >>>> : > >>>> Yes, I said about it at 07.19. > >>>> > >>>> > >> > http://apache-ignite-developers.2346864.n4.nabble.com/Improvements-for-new-security-approach-td42698.html#a42708 > >>>> And in my solution, I just transmitted security subjects for rest > >>> requests. > >>>> If you remove ATTR_SECURITY_SUBJECT_V2, it breaks compatibility > between > >>> old > >>>> versions and new. > >>>> > >>>> чт, 20 февр. 2020 г. в 15:56, Denis Garus : > >>>> > >>>>> Hi, Igniters! > >>>>> > >>>>> > >>>>> At present, a security subject id is assumed to be node id. > >>>>> > >>>>> But when we are dealing with thin client, JDBC or REST subject id is > >>>> random > >>>>> UUID. In this case, we cannot get the subject information on a remote > >>>> node, > >>>>> and we get problems like these [1], [2]. > >>>>> > >>>>> To fix the problem, we should spread the client session to the whole > >>>>> cluster. > >>>>> > >>>>> > >>>>> I want to suggest a solution to the problem. > >>>>> > >>>>> > >>>>> First, we should get subject information using GridSecurityProcessor. > >>>>> > >>>>> How GridSecurityProcessor will retrieve a subject data, it is up to > >>>> plugin > >>>>> developers. > >>>>> > >>>>> > >>>>> Second, we should get rid of the assumption that a subject id is a > >> node > >>>> id > >>>>> and remove the ATTR_SECURITY_SUBJECT_V2 attribute. > >>>>> > >>>>> > >>>>> I have prepared PoC PR [3] that: > >>>>> > >>>>> - places the existing logic of spreading security context to > >>>>> GridSecurityProcessor; > >>>>> > >>>>> - uses GridSecurityProcessor to get SecurityContext. > >>>>> > >>>>> > >>>>> > >>>>> 1. > >>>>> > >>>>> > >> > http://apache-ignite-developers.2346864.n4.nabble.com/JDBC-thin-client-incorrect-security-context-td45929.html > >>>>> 2. https://issues.apache.org/jira/browse/IGNITE-12589 > >>>>> 3. https://github.com/apache/ignite/pull/7375 > >>>>> > >> > >> -- > >> Best regards, > >>Andrey Kuznetsov. > >> > > >
Re: Unable to get the security context
Hi! It will be available in 2.8. сб, 22 февр. 2020 г. в 15:53, VeenaMithare : > HI Denis, > > Which version of Apache Ignite are the changes you mention in the comment( > security context always not null ) above available with ? In 2.7.6 I do get > security context as null in authorize method. > > regards, > Veena. > > > > -- > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/ >
Re: Security Subject of thin client on remote nodes
> I just transmitted security subjects for rest requests. SecurityContext has an unlimited size so we can get significant overhead. And we do not solve problems with other thin clients. >If you remove ATTR_SECURITY_SUBJECT_V2, it breaks compatibility between old versions and new. I suggest removing ATTR_SECURITY_SUBJECT_V2 from Ignite's codebase, but for compatibility, it can be used by a security plugin like in PoC. чт, 20 февр. 2020 г. в 16:47, Maksim Stepachev : > Yes, I said about it at 07.19. > > http://apache-ignite-developers.2346864.n4.nabble.com/Improvements-for-new-security-approach-td42698.html#a42708 > And in my solution, I just transmitted security subjects for rest requests. > > If you remove ATTR_SECURITY_SUBJECT_V2, it breaks compatibility between old > versions and new. > > чт, 20 февр. 2020 г. в 15:56, Denis Garus : > > > Hi, Igniters! > > > > > > At present, a security subject id is assumed to be node id. > > > > But when we are dealing with thin client, JDBC or REST subject id is > random > > UUID. In this case, we cannot get the subject information on a remote > node, > > and we get problems like these [1], [2]. > > > > To fix the problem, we should spread the client session to the whole > > cluster. > > > > > > I want to suggest a solution to the problem. > > > > > > First, we should get subject information using GridSecurityProcessor. > > > > How GridSecurityProcessor will retrieve a subject data, it is up to > plugin > > developers. > > > > > > Second, we should get rid of the assumption that a subject id is a node > id > > and remove the ATTR_SECURITY_SUBJECT_V2 attribute. > > > > > > I have prepared PoC PR [3] that: > > > > - places the existing logic of spreading security context to > > GridSecurityProcessor; > > > > - uses GridSecurityProcessor to get SecurityContext. > > > > > > > >1. > > > > > http://apache-ignite-developers.2346864.n4.nabble.com/JDBC-thin-client-incorrect-security-context-td45929.html > >2. https://issues.apache.org/jira/browse/IGNITE-12589 > >3. https://github.com/apache/ignite/pull/7375 > > >
Security Subject of thin client on remote nodes
Hi, Igniters! At present, a security subject id is assumed to be node id. But when we are dealing with thin client, JDBC or REST subject id is random UUID. In this case, we cannot get the subject information on a remote node, and we get problems like these [1], [2]. To fix the problem, we should spread the client session to the whole cluster. I want to suggest a solution to the problem. First, we should get subject information using GridSecurityProcessor. How GridSecurityProcessor will retrieve a subject data, it is up to plugin developers. Second, we should get rid of the assumption that a subject id is a node id and remove the ATTR_SECURITY_SUBJECT_V2 attribute. I have prepared PoC PR [3] that: - places the existing logic of spreading security context to GridSecurityProcessor; - uses GridSecurityProcessor to get SecurityContext. 1. http://apache-ignite-developers.2346864.n4.nabble.com/JDBC-thin-client-incorrect-security-context-td45929.html 2. https://issues.apache.org/jira/browse/IGNITE-12589 3. https://github.com/apache/ignite/pull/7375
Re: JDBC thin client incorrect security context
Hi! That is a known issue [1]. > is there any workaround to get this information? I think, unfortunately, no. 1. https://issues.apache.org/jira/browse/IGNITE-12589 пн, 17 февр. 2020 г. в 10:02, VeenaMithare : > Hi , > > Hi , > > We have built a security and audit plugin for security of our ignite > cluster. We are unable to get the right audit information i.e. we are > unable > to get the right subject for users logged in through dbeaver ( jdbc thin > client. ). This is because the subjectid associated with the "CACHE_PUT" > event when an update is triggered by the jdbc thin client, contains the > uuid > of the node that executed the update rather than the logged in jdbc thin > client user. > > If this is a limitation with the current version of ignite, is there any > workaround to get this information ? > > This was discussed in the 'Ignite users' group > > http://apache-ignite-users.70518.x6.nabble.com/JDBC-thin-client-incorrect-security-context-td31354.html > > And I was advised to continue my conversation here. > > Andrei mentions that the uuid associated with the event should be that of > the jdbc client( in our case the logged in dbeaver user ). But I notice > that > the event always contains the uuid of the node where the query gets > executed > . > > I do manage to get the right jdbc client from the authenticationcontext in > the authorize and form the right securitycontext. But this doesnt reflect > in > the generated cacheevent. > > Kindly guide. > regards, > Veena. > > > > -- > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/ >
[REVIEW] IGNITE-12344 Remote listener of IgniteMessaging has to run with appropriate SecurityContext
Hi, Igniters! I've prepared the PR[1] for the ticket[2], could somebody do a review? Thanks. 1. https://github.com/apache/ignite/pull/7338 2. https://issues.apache.org/jira/browse/IGNITE-12344
Re: [MTCGA]: new failures in builds [4973730] needs to be handled
Hello! I've created the task [1]. 1. https://issues.apache.org/jira/browse/IGNITE-12611 чт, 30 янв. 2020 г. в 20:56, : > Hi Igniters, > > I've detected some new issue on TeamCity to be handled. You are more than > welcomed to help. > > If your changes can lead to this failure(s): We're grateful that you were > a volunteer to make the contribution to this project, but things change and > you may no longer be able to finalize your contribution. > Could you respond to this email and indicate if you wish to continue and > fix test failures or step down and some committer may revert you commit. > > *New test failure in master EntryProcessorPermissionCheckTest.test > https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8=-59427578418496601=%3Cdefault%3E=testDetails > Changes may lead to failure were done by > - denis garus > https://ci.ignite.apache.org/viewModification.html?modId=897244 > > - Here's a reminder of what contributors were agreed to do > https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute > - Should you have any questions please contact > dev@ignite.apache.org > > Best Regards, > Apache Ignite TeamCity Bot > https://github.com/apache/ignite-teamcity-bot > Notification generated at 20:56:38 30-01-2020 >
[jira] [Created] (IGNITE-12611) EntryProcessorPermissionCheckTest.test: Test looks flaky
Denis Garus created IGNITE-12611: Summary: EntryProcessorPermissionCheckTest.test: Test looks flaky Key: IGNITE-12611 URL: https://issues.apache.org/jira/browse/IGNITE-12611 Project: Ignite Issue Type: Bug Reporter: Denis Garus Assignee: Denis Garus Test looks flaky.Test status change in build without changes: from failed to successful -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (IGNITE-12533) Remote security context tests refactoring.
Denis Garus created IGNITE-12533: Summary: Remote security context tests refactoring. Key: IGNITE-12533 URL: https://issues.apache.org/jira/browse/IGNITE-12533 Project: Ignite Issue Type: Improvement Reporter: Denis Garus To make tests more readable and robust, we should use the _AbstractRemoteSecurityContextCheckTest.Verifier#register(String)_ method in all related tests. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (IGNITE-12529) Getting local ignite instance inside ComputeJob#cancel
Denis Garus created IGNITE-12529: Summary: Getting local ignite instance inside ComputeJob#cancel Key: IGNITE-12529 URL: https://issues.apache.org/jira/browse/IGNITE-12529 Project: Ignite Issue Type: Bug Reporter: Denis Garus The Ignition#localIgnite method throws an exception while using inside the ComputeJob#cancel method. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (IGNITE-12500) IgniteProxy should be injected into non-system types only.
Denis Garus created IGNITE-12500: Summary: IgniteProxy should be injected into non-system types only. Key: IGNITE-12500 URL: https://issues.apache.org/jira/browse/IGNITE-12500 Project: Ignite Issue Type: Improvement Reporter: Denis Garus When the Ignite Sandbox is enabled, the GridResourceProxiedIgniteInjector should inject an IgniteProxy into non-system types only. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (IGNITE-12443) Document the Ignite Sandbox
Denis Garus created IGNITE-12443: Summary: Document the Ignite Sandbox Key: IGNITE-12443 URL: https://issues.apache.org/jira/browse/IGNITE-12443 Project: Ignite Issue Type: Task Components: documentation Reporter: Denis Garus Document how to configure and use the Ignite Sandbox. -- This message was sent by Atlassian Jira (v8.3.4#803005)
Re: Review needed for IGNITE-11410 Sandbox for user-defined code
Hello, Igniters! Alex Plehanov did the review, and I've reworked the implementation of the issue [1] that is part of the IEP-38 [2]. Could somebody else do a review of the PR [3]? Alex, thank you for the review! 1. https://issues.apache.org/jira/browse/IGNITE-11410 2. https://cwiki.apache.org/confluence/display/IGNITE/IEP-38%3A+Sandbox 3. https://github.com/apache/ignite/pull/6707 чт, 17 окт. 2019 г. в 16:21, Denis Garus : > Hello, Pavel! > > Thank you for the feedback! > > I've created IEP-38 that describes the Ignite Sandbox [1]. > > Yes, the issue requires documentation (there is the flag "Docs equired"), > but common practice is to write documentation in the end. > > >> 1) Why do you run resource injection through security and how it tested? > >> 2) Why do you check security at *dumpThreads* and *wrapThreadLoader > *methods? > >> These methods are needed only for internal node processes. > >> 4) There are suspicious security checks at: > >> CacheOperationContext:37 > >> GridCacheDefaultAffinityKeyMapper:86 > >> PageMemoryImpl:874 > >> I'm not following why they are needed. > > These questions have a common answer. > A user-defined code can call any operation through the public API of > Ignite. But he may don't have permissions to execute this operation > successfully. > For example, to put a value into a cache, it requires permissions for > accessing to reflection API and reading system property > IGNITE_ALLOW_ATOMIC_OPS_IN_TX. > In that case, we have to use AccessController#doPrirvelged call to exclude > a user-defined code from checking of permissions. > SecurityUtils#doPriveleged does calling AccessController#doPrirvelged a > more convenient way. > > >> 3) Have you tested security if a compute job is canceled? > You are right; we should add a test for the cancel case. > But, for now, we have the issue [2] with the current SecurityContext for > the canceling of ComputeJob. > > 1. https://cwiki.apache.org/confluence/display/IGNITE/IEP-38%3A+Sandbox > 2. https://issues.apache.org/jira/browse/IGNITE-12300 > > пн, 14 окт. 2019 г. в 16:16, Pavel Kovalenko : > >> Denis, >> >> The idea of having a sandbox for running a user-defined code is useful, >> but >> I don't fully understand the implementation approach. >> There is no detailed description of the ticket about what public API >> methods or configuration parameters should be covered. >> There is no description of what have done in initial PR and how. >> First of all, there should be an umbrella ticket that should contain all >> public API points and configuration parameters where used defined code may >> be run. >> Without a full list of all possible user-defined code injections, we can't >> track what was covered and where are a possible security lacks. >> I've checked PR and I have the following questions: >> 1) Why do you run resource injection through security and how it tested? >> 2) Why do you check security at *dumpThreads* and *wrapThreadLoader >> *methods? >> These methods are needed only for internal node processes. >> 3) Have you tested security if a compute job is canceled? >> 4) There are suspicious security checks at: >> CacheOperationContext:37 >> GridCacheDefaultAffinityKeyMapper:86 >> PageMemoryImpl:874 >> I'm not following why they are needed. >> >> >> >> >> пн, 14 окт. 2019 г. в 12:19, Anton Vinogradov : >> >> > Fully agree with the benchmark's importance. >> > Currently, we're not able to perform proper benchmarking. >> > Slava, Is it possible to ask you to check the solution using GridGain's >> > benchmarking environment? >> > >> > On Mon, Oct 14, 2019 at 12:07 PM Вячеслав Коптилин < >> > slava.kopti...@gmail.com> >> > wrote: >> > >> > > Hello Anton, >> > > >> > > > We should avoid heavy merges if possible. >> > > Why it should be avoided? To be honest, I don't see any reason for >> that. >> > > Every pull request can be and should be reviewed when it is done and >> > ready >> > > to be merged into the epic branch (IEP branch). >> > > So, the final review of the entire IEP is just a technical/trivial >> task, >> > in >> > > my opinion. >> > > >> > > If I am not mistaken, we are at the stage of preparing a new release >> > (2.8), >> > > right? >> > > And we are trying to add a new feature that may impact the >> performance. >> > > For example, affinity function,
Re: Thin client: compute support
Hello, Sergey! >> 3. Ignite doesn't have roles/authorization functionality for now. I can't agree with you. We already have authorization functionality in Ignite and for a thin client too [1]. But, compute support for a thin client requires some additional efforts to get an appropriate SecurityContext on a remote node. The list of tasks allowed for subjects, including thin clients, is the area of responsibility of GridSecurityProcessor [2]. 1. org.apache.ignite.internal.processors.security.client.ThinClientPermissionCheckTest 2. org.apache.ignite.internal.processors.security.GridSecurityProcessor чт, 21 нояб. 2019 г. в 12:41, Pavel Tupitsyn : > Good points, Sergey. > Maybe you are right, and Java-based compute without peer deployment is a > good first step for thin clients. > > On Thu, Nov 21, 2019 at 12:32 PM Sergey Kozlov > wrote: > > > Hi Pavel > > > > On Thu, Nov 21, 2019 at 11:30 AM Pavel Tupitsyn > > wrote: > > > > > 1. I believe that Cluster operations for Thin Client protocol are > already > > > in the works > > > by Alexandr Shapkin. Can't find the ticket though. > > > Alexandr, can you please confirm and attach the ticket number? > > > > > > 2. Proposed changes will work only for Java tasks that are already > > deployed > > > on server nodes. > > > This is mostly useless for other thin clients we have (Python, PHP, > .NET, > > > C++). > > > > > > > I don't guess so. The task (execution) is a way to implement own layer > for > > the thin client application. > > > > > > > We should think of a way to make this useful for all clients. > > > For example, we may allow sending tasks in some scripting language like > > > Javascript. > > > Thoughts? > > > > > > > The arbitrary code execution from a remote client must be protected > > from malicious code. > > I don't know how it could be designed but without that we open the hole > to > > kill cluster. > > > > > > > > > > On Thu, Nov 21, 2019 at 11:21 AM Sergey Kozlov > > > wrote: > > > > > > > Hi Alex > > > > > > > > The idea is great. But I have some concerns that probably should be > > taken > > > > into account for design: > > > > > > > >1. We need to have the ability to stop a task execution, smth like > > > >OP_COMPUTE_CANCEL_TASK operation (client to server) > > > >2. What's about task execution timeout? It may help to the cluster > > > >survival for buggy tasks > > > >3. Ignite doesn't have roles/authorization functionality for now. > > But > > > a > > > >task is the risky operation for cluster (for security reasons). > > Could > > > we > > > >add for Ignite configuration new options: > > > > - Explicit turning on for compute task support for thin > protocol > > > > (disabled by default) for whole cluster > > > > - Explicit turning on for compute task support for a node > > > > - The list of task names (classes) allowed to execute by thin > > > client. > > > >4. Support the labeling for task that may help to investigate > issues > > > on > > > >cluster (the idea from IEP-34 [1]) > > > > > > > > 1. > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/IGNITE/IEP-34+Thin+client%3A+transactions+support > > > > > > > > > > > > > > > > On Thu, Nov 21, 2019 at 10:58 AM Alex Plehanov < > > plehanov.a...@gmail.com> > > > > wrote: > > > > > > > > > Hello, Igniters! > > > > > > > > > > I have plans to start implementation of Compute interface for > Ignite > > > thin > > > > > client and want to discuss features that should be implemented. > > > > > > > > > > We already have Compute implementation for binary-rest clients > > > > > (GridClientCompute), which have the following functionality: > > > > > - Filtering cluster nodes (projection) for compute > > > > > - Executing task by the name > > > > > > > > > > I think we can implement this functionality in a thin client as > well. > > > > > > > > > > First of all, we need some operation types to request a list of all > > > > > available nodes and probably node attributes (by a list of nodes). > > Node > > > > > attributes will be helpful if we will decide to implement analog of > > > > > ClusterGroup#forAttribute or ClusterGroup#forePredicate methods in > > the > > > > thin > > > > > client. Perhaps they can be requested lazily. > > > > > > > > > > From the protocol point of view there will be two new operations: > > > > > > > > > > OP_CLUSTER_GET_NODES > > > > > Request: empty > > > > > Response: long topologyVersion, int minorTopologyVersion, int > > > nodesCount, > > > > > for each node set of node fields (UUID nodeId, Object or String > > > > > consistentId, long order, etc) > > > > > > > > > > OP_CLUSTER_GET_NODE_ATTRIBUTES > > > > > Request: int nodesCount, for each node: UUID nodeId > > > > > Response: int nodesCount, for each node: int attributesCount, for > > each > > > > node > > > > > attribute: String name, Object value > > > > > > > > > > To execute tasks we need something like these methods in
Re: Review for IGNITE-12300
Hi, Ilya! I've fixed the flaky test. Could you have a look? Thank you! чт, 7 нояб. 2019 г. в 17:12, Ilya Kasnacheev : > Hello! > > When I run this test, each case takes 10-40 seconds of hardcore disk usage > for some reason. > > Oops, sorry. I have just commented your ticket. > > Regards, > -- > Ilya Kasnacheev > > > чт, 7 нояб. 2019 г. в 16:18, Denis Garus : > > > >> but keep data in-memory? > > > > I don't use any data in tests for this issue; only compute task. > > > > > > >> I have commented JIRA with example of test failure. > > > > Unfortunately, I don't see your comment in JIRA [1]. > > > > > > 1. https://issues.apache.org/jira/browse/IGNITE-12300 > > > > чт, 7 нояб. 2019 г. в 15:45, Ilya Kasnacheev >: > > > > > Hello! > > > > > > I can see that, but for some reason I can see a lot of disk I/O while > > > running this test. Is it possible to only use persistence for auth > > > purposes, but keep data in-memory? > > > > > > I have commented JIRA with example of test failure. > > > > > > Regards, > > > -- > > > Ilya Kasnacheev > > > > > > > > > чт, 7 нояб. 2019 г. в 15:36, Denis Garus : > > > > > > > Hi, Ilya! > > > > > > > > Thank you for the review! > > > > > > > > > For some reason, when I run it locally, it starts to use > persistence, > > > do > > > > > you have ideas why that would happen? > > > > > > > > The reason is "Authentication can be enabled only for cluster with > > > enabled > > > > persistence." [1] > > > > > > > > > It also seems to me that > > > > > > > > > > > > > > > > > > > org.apache.ignite.internal.processors.security.compute.closure.ComputeTaskCancelRemoteSecurityContextCheckTest#testClientTaskInitatorCancelOnSrvNode > > > > > is flaky. > > > > > > > > I have started this test 25 times, and it is green. Could you please > > > > clarify why do you think the test is flaky? > > > > > > > > > > > > 1. > > > > > > > > > > > > > > https://github.com/apache/ignite/blob/720706d794e4935779cc2531462595032a547852/modules/core/src/main/java/org/apache/ignite/internal/processors/authentication/IgniteAuthenticationProcessor.java#L158 > > > > > > > > чт, 7 нояб. 2019 г. в 15:02, Ilya Kasnacheev < > > ilya.kasnach...@gmail.com > > > >: > > > > > > > > > Hello! > > > > > > > > > > I will re-run tests against fresh master, and then commit if they > > pass. > > > > > > > > > > For some reason, when I run it locally, it starts to use > persistence, > > > do > > > > > you have ideas why that would happen? > > > > > > > > > > It also seems to me that > > > > > > > > > > > > > > > > > > > > org.apache.ignite.internal.processors.security.compute.closure.ComputeTaskCancelRemoteSecurityContextCheckTest#testClientTaskInitatorCancelOnSrvNode > > > > > is flaky. > > > > > > > > > > Regards, > > > > > -- > > > > > Ilya Kasnacheev > > > > > > > > > > > > > > > чт, 7 нояб. 2019 г. в 12:10, Denis Garus : > > > > > > > > > > > Hello, Igniters! > > > > > > > > > > > > I've raised the PR [1] for the issue [2]. > > > > > > Could somebody review it? > > > > > > > > > > > > 1. https://github.com/apache/ignite/pull/7017 > > > > > > 2. https://issues.apache.org/jira/browse/IGNITE-12300 > > > > > > > > > > > > > > > > > > > > >
Re: Review for IGNITE-12300
>> but keep data in-memory? I don't use any data in tests for this issue; only compute task. >> I have commented JIRA with example of test failure. Unfortunately, I don't see your comment in JIRA [1]. 1. https://issues.apache.org/jira/browse/IGNITE-12300 чт, 7 нояб. 2019 г. в 15:45, Ilya Kasnacheev : > Hello! > > I can see that, but for some reason I can see a lot of disk I/O while > running this test. Is it possible to only use persistence for auth > purposes, but keep data in-memory? > > I have commented JIRA with example of test failure. > > Regards, > -- > Ilya Kasnacheev > > > чт, 7 нояб. 2019 г. в 15:36, Denis Garus : > > > Hi, Ilya! > > > > Thank you for the review! > > > > > For some reason, when I run it locally, it starts to use persistence, > do > > > you have ideas why that would happen? > > > > The reason is "Authentication can be enabled only for cluster with > enabled > > persistence." [1] > > > > > It also seems to me that > > > > > > > > org.apache.ignite.internal.processors.security.compute.closure.ComputeTaskCancelRemoteSecurityContextCheckTest#testClientTaskInitatorCancelOnSrvNode > > > is flaky. > > > > I have started this test 25 times, and it is green. Could you please > > clarify why do you think the test is flaky? > > > > > > 1. > > > > > https://github.com/apache/ignite/blob/720706d794e4935779cc2531462595032a547852/modules/core/src/main/java/org/apache/ignite/internal/processors/authentication/IgniteAuthenticationProcessor.java#L158 > > > > чт, 7 нояб. 2019 г. в 15:02, Ilya Kasnacheev >: > > > > > Hello! > > > > > > I will re-run tests against fresh master, and then commit if they pass. > > > > > > For some reason, when I run it locally, it starts to use persistence, > do > > > you have ideas why that would happen? > > > > > > It also seems to me that > > > > > > > > > org.apache.ignite.internal.processors.security.compute.closure.ComputeTaskCancelRemoteSecurityContextCheckTest#testClientTaskInitatorCancelOnSrvNode > > > is flaky. > > > > > > Regards, > > > -- > > > Ilya Kasnacheev > > > > > > > > > чт, 7 нояб. 2019 г. в 12:10, Denis Garus : > > > > > > > Hello, Igniters! > > > > > > > > I've raised the PR [1] for the issue [2]. > > > > Could somebody review it? > > > > > > > > 1. https://github.com/apache/ignite/pull/7017 > > > > 2. https://issues.apache.org/jira/browse/IGNITE-12300 > > > > > > > > > >
Re: Review for IGNITE-12300
Hi, Ilya! Thank you for the review! > For some reason, when I run it locally, it starts to use persistence, do > you have ideas why that would happen? The reason is "Authentication can be enabled only for cluster with enabled persistence." [1] > It also seems to me that > org.apache.ignite.internal.processors.security.compute.closure.ComputeTaskCancelRemoteSecurityContextCheckTest#testClientTaskInitatorCancelOnSrvNode > is flaky. I have started this test 25 times, and it is green. Could you please clarify why do you think the test is flaky? 1. https://github.com/apache/ignite/blob/720706d794e4935779cc2531462595032a547852/modules/core/src/main/java/org/apache/ignite/internal/processors/authentication/IgniteAuthenticationProcessor.java#L158 чт, 7 нояб. 2019 г. в 15:02, Ilya Kasnacheev : > Hello! > > I will re-run tests against fresh master, and then commit if they pass. > > For some reason, when I run it locally, it starts to use persistence, do > you have ideas why that would happen? > > It also seems to me that > > org.apache.ignite.internal.processors.security.compute.closure.ComputeTaskCancelRemoteSecurityContextCheckTest#testClientTaskInitatorCancelOnSrvNode > is flaky. > > Regards, > -- > Ilya Kasnacheev > > > чт, 7 нояб. 2019 г. в 12:10, Denis Garus : > > > Hello, Igniters! > > > > I've raised the PR [1] for the issue [2]. > > Could somebody review it? > > > > 1. https://github.com/apache/ignite/pull/7017 > > 2. https://issues.apache.org/jira/browse/IGNITE-12300 > > >
Review for IGNITE-12300
Hello, Igniters! I've raised the PR [1] for the issue [2]. Could somebody review it? 1. https://github.com/apache/ignite/pull/7017 2. https://issues.apache.org/jira/browse/IGNITE-12300
[jira] [Created] (IGNITE-12345) Remote listener of IgniteMessaging has to run inside the Ignite Sandbox.
Denis Garus created IGNITE-12345: Summary: Remote listener of IgniteMessaging has to run inside the Ignite Sandbox. Key: IGNITE-12345 URL: https://issues.apache.org/jira/browse/IGNITE-12345 Project: Ignite Issue Type: Task Reporter: Denis Garus Remote listener of IgniteMessaging has to run on a remote node inside the Ignite Sandbox if it is turned on. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (IGNITE-12344) Remote listener of IgniteMessaging has to run with appropriate SecurityContext.
Denis Garus created IGNITE-12344: Summary: Remote listener of IgniteMessaging has to run with appropriate SecurityContext. Key: IGNITE-12344 URL: https://issues.apache.org/jira/browse/IGNITE-12344 Project: Ignite Issue Type: Bug Reporter: Denis Garus Remote listener of IgniteMessaging has to run on a remote node with the SecurityContext of the initiator node. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (IGNITE-12343) Remote filter and transformer of ContinuousQueries have to run inside the Ignite Sandbox
Denis Garus created IGNITE-12343: Summary: Remote filter and transformer of ContinuousQueries have to run inside the Ignite Sandbox Key: IGNITE-12343 URL: https://issues.apache.org/jira/browse/IGNITE-12343 Project: Ignite Issue Type: Task Reporter: Denis Garus Remote filter and transformer of ContinuousQueries have to run on a remote node inside the Ignite Sandbox if it is turned on. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (IGNITE-12342) Continuous Queries: Remote filter and transformer have to run with appropriate SecurityContext.
Denis Garus created IGNITE-12342: Summary: Continuous Queries: Remote filter and transformer have to run with appropriate SecurityContext. Key: IGNITE-12342 URL: https://issues.apache.org/jira/browse/IGNITE-12342 Project: Ignite Issue Type: Bug Reporter: Denis Garus Remote filter and transformer of ContinuousQueries have to run on a remote node with the SecurityContext of the initiator node. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (IGNITE-12341) Sandbox: Adding tests for ComputeJob#cancel
Denis Garus created IGNITE-12341: Summary: Sandbox: Adding tests for ComputeJob#cancel Key: IGNITE-12341 URL: https://issues.apache.org/jira/browse/IGNITE-12341 Project: Ignite Issue Type: Task Reporter: Denis Garus We have to have tests that check the ComputeJob#cancel method is executed inside the Ignite Sandbox on a remote node. -- This message was sent by Atlassian Jira (v8.3.4#803005)
Re: Write access to Apache Ignite Confluence
Hello, Dmitriy! I've got the write access. Thank you! пн, 21 окт. 2019 г. в 22:03, Dmitriy Pavlov : > I found someone has granted permission to Denis Garus. Could you confirm > everything is ok? > > вт, 15 окт. 2019 г. в 10:12, Denis Garus : > > > Hi, Igniters! > > > > I'd like to add a new wiki page with IEP for the Sandbox feature. > > > > Could someone grant me (Denis Garus) write permission? > > >
Re: Review needed for IGNITE-11410 Sandbox for user-defined code
г. в 09:09, Anton Vinogradov : > > > > > > > Folks, > > > > > > > > We should avoid heavy merges if possible. > > > > I'm ok with IEP to keep tasks properly, but strictly against > all-in-one > > > > "+27000,-18200" merges. > > > > This task implements Sandbox (API + core) which covered by tests and > by > > > > some integrations with existing components, which is enough to merge. > > > > The most important thing here is that we will be able to speed-up > > Sandbox > > > > coverage development once its core menged to the master. > > > > > > > > On Fri, Oct 11, 2019 at 5:41 PM Вячеслав Коптилин < > > > > slava.kopti...@gmail.com> > > > > wrote: > > > > > > > > > Hi Denis, > > > > > > > > > > In my humble opinion, the security (the sandbox feature is about > > > > security, > > > > > right?) either covers all APIs/subsystems or not. > > > > > Security should work always and everywhere otherwise it is not > > security > > > > :) > > > > > > > > > > > From my point, we should divide the sandbox and features that use > > it. > > > > > > Also, I added in the main features of Ignite (cache and compute) > > the > > > > > sandbox calls. > > > > > And at this point, you mixed both in the same pull-request. > > > > > > > > > > > I don't see any problem to have the sandbox in the master branch > > and > > > > > implement covering for existing and new features if needed. > > > > > On the other hand, this approach leads to ... > > > > > ignite-123 [IEP-X] introduces new cool API > > > > > ignite-124 [IEP-X] improved cool API > > > > > ignite-125 [IEP-X] fixed a bug > > > > > ignite-126 [IEP-X] fixed performance drop > > > > > ignite-127 [IEP-X] Cache API uses new API > > > > > ignite-127 [IEP-X] Compute grid uses new API > > > > > ... > > > > > > > > > > Why should it be a part of the master branch history? All these > > things > > > > can > > > > > be done on the feature branch, I think. Anyway, it is up to you. > > > > > > > > > > Thanks, > > > > > S. > > > > > > > > > > пт, 11 окт. 2019 г. в 16:31, Denis Garus : > > > > > > > > > > > From my point, we should divide the sandbox and features that use > > it. > > > > > > The sandbox is fully implemented and has needed tests. > > > > > > > > > > > > Also, I added in the main features of Ignite (cache and compute) > > the > > > > > > sandbox calls. > > > > > > > > > > > > I don't see any problem to have the sandbox in the master branch > > > > > > and implement covering for existing and new features if needed. > > > > > > > > > > > > пт, 11 окт. 2019 г. в 15:21, Вячеслав Коптилин < > > > > slava.kopti...@gmail.com > > > > > >: > > > > > > > > > > > > > Hi Denis, > > > > > > > > > > > > > > Yep, I understand the scope of the ticket, but... I think it is > > > not a > > > > > > good > > > > > > > idea to merge partly implemented feature(s) into the master > > branch. > > > > > > > Especially, at this moment. We are at the stage of preparing a > > new > > > > > > release > > > > > > > and I doubt that all improvements, tests (unit tests, > integration > > > > > tests, > > > > > > > and performance tests) can be implemented before the release > > branch > > > > is > > > > > > cut > > > > > > > off. > > > > > > > Personally, I would prefer to create an epic/feature branch for > > > these > > > > > > > activities. In that case, we can implement a feature step by > step > > > and > > > > > > merge > > > > > > > it into the master branch once all components are covered. > > > > > > > > > > > > > > > But, sure, we should execute any user-defined code in the > > sandbox > > > > on > &g
[jira] [Created] (IGNITE-12300) ComputeJob#cancel executes with wrong SecurityContext
Denis Garus created IGNITE-12300: Summary: ComputeJob#cancel executes with wrong SecurityContext Key: IGNITE-12300 URL: https://issues.apache.org/jira/browse/IGNITE-12300 Project: Ignite Issue Type: Bug Reporter: Denis Garus ComputeJob#cancel executes with context of current node rather then should be executed with context of node that initiate ComputeJob. -- This message was sent by Atlassian Jira (v8.3.4#803005)
Write access to Apache Ignite Confluence
Hi, Igniters! I'd like to add a new wiki page with IEP for the Sandbox feature. Could someone grant me (Denis Garus) write permission?
Re: Review needed for IGNITE-11410 Sandbox for user-defined code
>From my point, we should divide the sandbox and features that use it. The sandbox is fully implemented and has needed tests. Also, I added in the main features of Ignite (cache and compute) the sandbox calls. I don't see any problem to have the sandbox in the master branch and implement covering for existing and new features if needed. пт, 11 окт. 2019 г. в 15:21, Вячеслав Коптилин : > Hi Denis, > > Yep, I understand the scope of the ticket, but... I think it is not a good > idea to merge partly implemented feature(s) into the master branch. > Especially, at this moment. We are at the stage of preparing a new release > and I doubt that all improvements, tests (unit tests, integration tests, > and performance tests) can be implemented before the release branch is cut > off. > Personally, I would prefer to create an epic/feature branch for these > activities. In that case, we can implement a feature step by step and merge > it into the master branch once all components are covered. > > > But, sure, we should execute any user-defined code in the sandbox on a > remote node. Feel free to create issues. > will do. > > Thanks, > S. > > пт, 11 окт. 2019 г. в 14:52, Denis Garus : > > > Hello, Slava! > > > > The scope of the issue is limited by the following features: > > > >- StreamReceiver for DataStreamer; > >- EntryProcessor; > >- ComputeJob; > >- filter and transformer for ScanQuery. > > > > But, sure, we should execute any user-defined code in the sandbox on a > > remote node. > > Feel free to create issues. > > > > Thanks for the feedback! > > > > пт, 11 окт. 2019 г. в 13:26, Вячеслав Коптилин >: > > > > > Hello Denis, Anton, > > > > > > Could you please clarify the following aspect? Do we need the same > > > changes/capabilities related to Continuous Queries, Disco listeners, > > > CacheStore Factories etc? > > > > > > Thanks, > > > S. > > > > > > пт, 11 окт. 2019 г. в 12:24, Anton Vinogradov : > > > > > > > Folks, > > > > > > > > As a prereviewer, I'd like to say that the solution looks good to me, > > but > > > > fresh eyes would be good. > > > > > > > > On Fri, Oct 11, 2019 at 9:40 AM Denis Garus > > wrote: > > > > > > > > > Hello, Igniters! > > > > > > > > > > I've raised the PR [1] with the sandbox for AI [2]. > > > > > Could somebody review it? > > > > > > > > > > If you have questions and prefer the Slack, I've created the > channel > > > [3]. > > > > > > > > > > 1. https://github.com/apache/ignite/pull/6707 > > > > > 2. https://issues.apache.org/jira/browse/IGNITE-11410 > > > > > 3. https://app.slack.com/client/T4S1WH2J3/CP8JER880 > > > > > > > > > > > > > > >
Re: Review needed for IGNITE-11410 Sandbox for user-defined code
Hello, Slava! The scope of the issue is limited by the following features: - StreamReceiver for DataStreamer; - EntryProcessor; - ComputeJob; - filter and transformer for ScanQuery. But, sure, we should execute any user-defined code in the sandbox on a remote node. Feel free to create issues. Thanks for the feedback! пт, 11 окт. 2019 г. в 13:26, Вячеслав Коптилин : > Hello Denis, Anton, > > Could you please clarify the following aspect? Do we need the same > changes/capabilities related to Continuous Queries, Disco listeners, > CacheStore Factories etc? > > Thanks, > S. > > пт, 11 окт. 2019 г. в 12:24, Anton Vinogradov : > > > Folks, > > > > As a prereviewer, I'd like to say that the solution looks good to me, but > > fresh eyes would be good. > > > > On Fri, Oct 11, 2019 at 9:40 AM Denis Garus wrote: > > > > > Hello, Igniters! > > > > > > I've raised the PR [1] with the sandbox for AI [2]. > > > Could somebody review it? > > > > > > If you have questions and prefer the Slack, I've created the channel > [3]. > > > > > > 1. https://github.com/apache/ignite/pull/6707 > > > 2. https://issues.apache.org/jira/browse/IGNITE-11410 > > > 3. https://app.slack.com/client/T4S1WH2J3/CP8JER880 > > > > > >
[jira] [Created] (IGNITE-12283) Access restriction to IgniteKernal
Denis Garus created IGNITE-12283: Summary: Access restriction to IgniteKernal Key: IGNITE-12283 URL: https://issues.apache.org/jira/browse/IGNITE-12283 Project: Ignite Issue Type: Task Reporter: Denis Garus IgniteKernal allows a user-defined code to get access to the internal features of Ignite. That behavior leads to security lack. We must encapsulate _IgniteKernal_ to restrict access to it from user-defined code. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (IGNITE-12282) Access restriction to the internal package of Ignite
Denis Garus created IGNITE-12282:
Summary: Access restriction to the internal package of Ignite
Key: IGNITE-12282
URL: https://issues.apache.org/jira/browse/IGNITE-12282
Project: Ignite
Issue Type: Task
Reporter: Denis Garus
A user-defined code shouldn't have access to _org.apache.ignite.internal_.*
that is the internal package of Ignite.
To restrict a user-defined code, we need to set the package name to the
_package.access_ security property.
To grand access to a package, we have to use _accessClassInPackage.\{package
name}_ runtime permission.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
Review needed for IGNITE-11410 Sandbox for user-defined code
Hello, Igniters! I've raised the PR [1] with the sandbox for AI [2]. Could somebody review it? If you have questions and prefer the Slack, I've created the channel [3]. 1. https://github.com/apache/ignite/pull/6707 2. https://issues.apache.org/jira/browse/IGNITE-11410 3. https://app.slack.com/client/T4S1WH2J3/CP8JER880
Re: Improvements for new security approach.
There is the #ignite-security Slack channel; we can use it for discussion. вт, 1 окт. 2019 г. в 09:14, Anton Vinogradov : > Folks, > > Could you please create a slack channel to discuss this in an effective > way? > > On Mon, Sep 30, 2019 at 5:36 PM Denis Garus wrote: > > > >>As a result, you can't load security on demand. > > > > Why? > > What is the difference between sending SecurityContext with every job's > > request and sending SecurityContext once when remote node demand it? > > > > пн, 30 сент. 2019 г. в 17:25, Maksim Stepachev < > maksim.stepac...@gmail.com > > >: > > > > > I suppose that code works only with requests are made from > > > GridRestProcessor (It isn't a client, I call it like a fake client). > As a > > > result, you can't load security on demand. If you want to do it, you > > > should transmit HTTP session and backward address of a node which > > > received REST request. > > > > > > пн, 30 сент. 2019 г. в 16:16, Denis Garus : > > > > > >> >>>> Subject's size is unlimited, that can lead to a dramatic increase > > in > > >> traffic between nodes. > > >> >>I added network optimization for this case. I add a subject in the > > case > > >> when ctx.discovery().node(secSubjId) == null. > > >> > > >> Yes, this optimization is good, but we have to send SecurityContext > > >> whenever a client starts a job. > > >> Why? > > >> > > >> A better solution would be to send SecurityContext on demand. > > >> > > >> Imagine the following scenario. > > >> A client connects to node A and starts a job that runs on remote node > B. > > >> IgniteSecurityProcessor on node B tries to find SecurityContext by > > >> subjectId. > > >> If IgniteSecurityProcessor fails, then it sends SecurityContextRequest > > to > > >> node A and gets required SecurityContext > > >> that afterward puts to the IgniteSecurityProcessor's cache on node B. > > >> The next request to run a job on node B with this subjectId doesn't > > >> require SecurityContext transmission. > > >> > > >> SecurityContextResponse can contain some additional information, for > > >> example, > > >> time-to-live of SecurityContext before eviction SecurityContext from > the > > >> IgniteSecurityProcessor's cache. > > >> > > >> пт, 27 сент. 2019 г. в 15:20, Maksim Stepachev < > > >> maksim.stepac...@gmail.com>: > > >> > > >>> I finished with fixes: > > >>> https://issues.apache.org/jira/browse/IGNITE-11992 > > >>> > > >>> >> Subject's size is unlimited, that can lead to a dramatic increase > in > > >>> traffic between nodes. > > >>> I added network optimization for this case. I add a subject in the > case > > >>> when ctx.discovery().node(secSubjId) == null. > > >>> > > >>> >> Also, we need to get rid of GridTaskThreadContextKey#TC_SUBJ_ID as > > >>> duplication of IgnitSecurity responsibility. > > >>> [2]Yes, we should rid of this. But in the next task, because I can't > > >>> merge it since 18 Jul 19:) > > >>> > > >>> [1] I aggry with you. > > >>> > > >>> > > >>> пт, 27 сент. 2019 г. в 11:42, Denis Garus : > > >>> > > >>>> Hello, Maksim! > > >>>> > > >>>> Thank you for your effort and interest in the security of Ignite. > > >>>> > > >>>> I would like you to pay attention to the discussion [1] and issue > [2]. > > >>>> It looks like not only task should execute in the current security > > >>>> context but all operations too, that is essential to determine a > > security > > >>>> id for events. > > >>>> Also, we need to get rid of GridTaskThreadContextKey#TC_SUBJ_ID as > > >>>> duplication of IgnitSecurity responsibility. > > >>>> I think your task is the right place to do that. > > >>>> What is your opinion? > > >>>> > > >>>> >>It's the reason why subject id isn't enough and we should transmit > > >>>> subject inside message for this case. > > >>>> There is a problem with this approach. > > >>>>
Re: JavaDoc for Event's subjectId methods
Thank you, Maksim! I think that the description should look like: "Gets security subject ID initiated this task event if IgniteSecurity is enabled, otherwise returns null. This property is not available for GridEventType#EVT_TASK_SESSION_ATTR_SET task event." Please pay close attention that described behavior is confirmed by tests. пн, 30 сент. 2019 г. в 13:11, Maksim Stepachev : > Denis, > > I added it in my ticket and pull-request. Should I change the only first > sentence or full comment? > > пн, 30 сент. 2019 г. в 11:27, Denis Garus : > >> Hello! >> >> I suggested to Maksim Stepachev include these changes in the scope of >> your thicket [1] >> and it looks like he agreed [2]. >> >> Maksim Stepachev, could you please reflect JavaDoc and behavior changes >> of events in your ticket? >> >> 1. https://issues.apache.org/jira/browse/IGNITE-11992 >> 2. >> http://apache-ignite-developers.2346864.n4.nabble.com/Improvements-for-new-security-approach-td42698.html >> >> пн, 30 сент. 2019 г. в 11:07, Ivan Pavlukhin : >> >>> Hi, >>> >>> Do we allow commits to master without a ticket? I can imagine only >>> reverts as an exception. >>> >>> Otherwise a ticket is a primary process item. Work description, >>> review, CI checks (we have a job checking javadocs). >>> >>> ср, 25 сент. 2019 г. в 01:15, Denis Magda : >>> > >>> > Denis, please feel free to go and edit the JavaDocs in place without a >>> > ticket. The changes suggested by you are reasonable. >>> > >>> > - >>> > Denis >>> > >>> > >>> > On Tue, Sep 24, 2019 at 3:55 AM Denis Garus >>> wrote: >>> > >>> > > Hello, Igniters! >>> > > >>> > > Some events contain the subjectId method, for example, >>> TaskEvent#subjectId. >>> > > The JavaDoc for this method is: >>> > > "Gets security subject ID initiated this task event, if available. >>> > > This property is not available for >>> GridEventType#EVT_TASK_SESSION_ATTR_SET >>> > > task event. >>> > > Subject ID will be set either to node ID or client ID initiated task >>> > > execution." >>> > > >>> > > I think It's wrong. The main point is a subject id doesn't have any >>> sense >>> > > if IgniteSecurity is disabled. >>> > > However, if IgniteSecurity is enabled, the method must return the >>> subject >>> > > id from the current security context. >>> > > Thus, the description (and behavior) of the method should be the >>> following: >>> > > Gets security subject ID initiated this task event if IgniteSecurity >>> is >>> > > enabled, otherwise returns null. >>> > > >>> > > The same is actual for CacheEvent, CacheQueryExecutedEvent and >>> > > CacheQueryReadEvent. >>> > > >>> > > If there are no objections, I am going to create a relevant issue in >>> Jira. >>> > > >>> >>> >>> >>> -- >>> Best regards, >>> Ivan Pavlukhin >>> >>
Re: Improvements for new security approach.
Hello, Maksim! Thank you for your effort and interest in the security of Ignite. I would like you to pay attention to the discussion [1] and issue [2]. It looks like not only task should execute in the current security context but all operations too, that is essential to determine a security id for events. Also, we need to get rid of GridTaskThreadContextKey#TC_SUBJ_ID as duplication of IgnitSecurity responsibility. I think your task is the right place to do that. What is your opinion? >>It's the reason why subject id isn't enough and we should transmit subject inside message for this case. There is a problem with this approach. Subject's size is unlimited, that can lead to a dramatic increase in traffic between nodes. 1. http://apache-ignite-developers.2346864.n4.nabble.com/JavaDoc-for-Event-s-subjectId-methods-td43663.html 2. https://issues.apache.org/jira/browse/IGNITE-9914 пт, 27 сент. 2019 г. в 08:38, Anton Vinogradov : > Maksim > > >> I want to fix 2-3-4 points under one ticket. > Please let me know once it's become ready to be reviewed. > > On Thu, Sep 26, 2019 at 5:18 PM Maksim Stepachev < > maksim.stepac...@gmail.com> > wrote: > > > Hi. > > > > Anton Vinogradov, > > > > I want to fix 2-3-4 points under one ticket. > > > > The first was fixed in the ticket: > > https://issues.apache.org/jira/browse/IGNITE-11094 > > Also, I aggry with you that 5-6 isn't required to ignite. > > > > Denis Garus, > > I made reproducer for point 3. Looks at the test from my pull-request: > > JettyRestPropagationSecurityContextTest > > > > https://github.com/apache/ignite/pull/6918 > > > > For point 2 you should apply GridRestProcessor from pr and set debug into > > VisorQueryUtils#scheduleQueryStart between > > ignite.context().closure().runLocalSafe and call: > > ignite.context().security().securityContext() > > > > > > For point 3, do action above and call: > > > ignite.context().discovery().node(ignite.context().security().securityContext().subject().id()) > > > > It returns null because this subject was created from the rest. It's the > > reason why subject id isn't enough and we should transmit subject inside > > message for this case. > > > > чт, 18 июл. 2019 г. в 12:45, Anton Vinogradov : > > > >> Maksim, > >> > >> Could you please split IGNITE-11992 to subtasks with proper > descriptions? > >> This will allow us to relocate discussion to the issues to solve each > >> problem properly. > >> > >> On Thu, Jul 18, 2019 at 11:57 AM Denis Garus > wrote: > >> > >> > Hello, Maksim! > >> > Thanks for your analysis! > >> > > >> > I have a few questions about your proposals. > >> > > >> > GridRestProcessor. > >> > AFAIK, when GridRestProcessor handle client request > >> > (GridRestProcessor#handleRequest) > >> > it process authentication (GridRestProcessor#authenticate) > >> > and then authorization of request (GridRestProcessor#authorize) inside > >> > client context. > >> > Can you give additional info about issues with GridRestProcessor from > 3 > >> and > >> > 4? Maybe you have a reproducer for the problem? > >> > > >> > NoOpIgniteSecurityProcessor. > >> > I think the case that you describe in 5 is not a bug. > >> > All nodes (client and server) must have security enabled or disabled. > >> > I can't imagine the case when it is not. > >> > > >> > ATTR_SECURITY_SUBJECT. > >> > I don't think that compatibility is needed here. If you will use node > >> with > >> > propagation security context to remote node and older nodes > >> > you can get subtle errors. > >> > > >> > чт, 18 июл. 2019 г. в 11:12, Maksim Stepachev < > >> maksim.stepac...@gmail.com > >> > >: > >> > > >> > > Hi, Ivan. > >> > > > >> > > Yes, I have. > >> > > https://issues.apache.org/jira/browse/IGNITE-11992 > >> > > > >> > > I'm waiting for a visa. > >> > > > >> > > > >> > > чт, 18 июл. 2019 г. в 11:09, Ivan Rakov : > >> > > > >> > >> Hello Max, > >> > >> > >> > >> Thanks for your analysis! > >> > >> > >> > >> Have you created a JIRA issue for discovered defects? > >> > >> > >> > >> Best Regards, > >> > &g
Re: Improvements for P2P class loading
Denis, yes you are right, there is a reason for this. It's the case when one of a participated node leaves a cluster and loader send a request to the next node. Also, the list of participated nodes is limited enough. Thus, the proposed improvement a) is wrong. But I think the point b) is still actual. Thank you for the feedback! ср, 25 сент. 2019 г. в 01:32, Denis Magda : > Denis, > > Why do we broadcast the request to all the nodes in case of scenario a)? > There should be a reason for that. > > Alex Goncharuk, do you remember we planned to revisit the P2P as part of > Ignite 3.0 to remove certain limitations? How about linking all the P2P > tasks together and create either an IEP or an umbrella ticket. > > - > Denis > > > On Tue, Sep 24, 2019 at 12:55 AM Denis Garus wrote: > > > Igniters! > > > > I would like to propose a few improvements for the P2P class loading > > feature in Ignite. > > These improvements have the aim to reduce the number of requests that may > > be needing to get a class on a remote node. > > > > a. We should send a request for a class to the node initiator firstly. > > Currently, GridDeploymentClassLoader sends a request for a class to all > > participated nodes one by one until it gets a successful response. > However, > > in most cases, the required byte code would be loaded from the node that > > initiates execution. To find out what is the node initiator, we can use > the > > way that we use to determine a security context to execute a user-defined > > code (see GridIoManager#invokeListener). If the node initiator doesn't > have > > a required class, we should ask other participated nodes as it is > > currently. > > > > b. Some classes in a single response. > > When a required class contains anonymous and/or inner classes, Ignite > tries > > to get these classes using separate requests. However, we can determine > > that case and send data, that we send anyway, in a single response as an > > answer for the first request. In this way, we may significantly reduce > the > > number of communications required for a class loading. > > > > What do you think? > > > > I would like to create a separate task for a. and b. > > >
JavaDoc for Event's subjectId methods
Hello, Igniters! Some events contain the subjectId method, for example, TaskEvent#subjectId. The JavaDoc for this method is: "Gets security subject ID initiated this task event, if available. This property is not available for GridEventType#EVT_TASK_SESSION_ATTR_SET task event. Subject ID will be set either to node ID or client ID initiated task execution." I think It's wrong. The main point is a subject id doesn't have any sense if IgniteSecurity is disabled. However, if IgniteSecurity is enabled, the method must return the subject id from the current security context. Thus, the description (and behavior) of the method should be the following: Gets security subject ID initiated this task event if IgniteSecurity is enabled, otherwise returns null. The same is actual for CacheEvent, CacheQueryExecutedEvent and CacheQueryReadEvent. If there are no objections, I am going to create a relevant issue in Jira.
Improvements for P2P class loading
Igniters! I would like to propose a few improvements for the P2P class loading feature in Ignite. These improvements have the aim to reduce the number of requests that may be needing to get a class on a remote node. a. We should send a request for a class to the node initiator firstly. Currently, GridDeploymentClassLoader sends a request for a class to all participated nodes one by one until it gets a successful response. However, in most cases, the required byte code would be loaded from the node that initiates execution. To find out what is the node initiator, we can use the way that we use to determine a security context to execute a user-defined code (see GridIoManager#invokeListener). If the node initiator doesn't have a required class, we should ask other participated nodes as it is currently. b. Some classes in a single response. When a required class contains anonymous and/or inner classes, Ignite tries to get these classes using separate requests. However, we can determine that case and send data, that we send anyway, in a single response as an answer for the first request. In this way, we may significantly reduce the number of communications required for a class loading. What do you think? I would like to create a separate task for a. and b.
Re: After IGNITE-12148 the Examples suite has unstable tests
Alexey, I run tests two times on your branch, all are green. Thank you, great job! пт, 13 сент. 2019 г. в 23:04, Alexey Zinoviev : > Fixed it, please have a look and test it > > https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_Examples?branch=%3Cdefault%3E > > > пт, 13 сент. 2019 г. в 21:16, Alexey Zinoviev : > > > Ok, got it, I've found a solution > > > > пт, 13 сент. 2019 г. в 21:09, Denis Garus : > > > >> Alexey, about agents was my assumption, and it looks like wrong. > >> I didn't dive so deep. > >> > >> пт, 13 сент. 2019 г. в 20:54, Alexey Zinoviev : > >> > >> > Could you help me recognize the difference between agents to exactly > >> > reproduce the issue and be sure that I fix it? > >> > > >> > > >> > пт, 13 сент. 2019 г. в 20:51, Alexey Zinoviev >: > >> > > >> > > Thank you, I'll try to fix it, ticket is here > >> > > https://issues.apache.org/jira/browse/IGNITE-12168 > >> > > > >> > > пт, 13 сент. 2019 г. в 20:41, Denis Garus : > >> > > > >> > >> Alexey, sure. > >> > >> My first build today is [1], and the last build is [2]. > >> > >> 28 tests became flaky. > >> > >> > >> > >> 1. > >> > >> > >> > >> > >> > > >> > https://ci.ignite.apache.org/viewLog.html?buildId=4594295=IgniteTests24Java8_Examples > >> > >> 2. > >> > >> > >> > >> > >> > > >> > https://ci.ignite.apache.org/viewLog.html?buildId=4595102=IgniteTests24Java8_Examples > >> > >> > >> > >> пт, 13 сент. 2019 г. в 20:25, Alexey Zinoviev < > >> zaleslaw@gmail.com>: > >> > >> > >> > >> > Dear @ Denis Garus > >> > >> > > >> > >> > Could you please what kind of tests became unstable? > >> > >> > Because I have no troubles with examples (and run them of course > >> > before > >> > >> > merging) and many builds have no troubles too > >> > >> > > >> > >> > Look at > >> > >> > > >> > >> > > >> > >> > >> > > >> > https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_Examples?branch=%3Cdefault%3E > >> > >> > - > >> > >> > it's broken on Scala component a few weeks ago > >> > >> > > >> > >> > Could you please send me exact link on the builds that were ran > by > >> > >> yourself > >> > >> > today > >> > >> > > >> > >> > But yes, the IGNITE-12148 could influence on many examples in ML > >> > module > >> > >> > because a few of resources were moved. > >> > >> > I will try to fix today or tomorrow (but I need more information > to > >> > >> > reproduce the situation) > >> > >> > > >> > >> > > >> > >> > пт, 13 сент. 2019 г. в 19:52, Denis Magda : > >> > >> > > >> > >> > > Alex Zinoviev, could you please double-check and confirm if > >> > >> IGNITE-12148 > >> > >> > > affects the test suite? > >> > >> > > > >> > >> > > - > >> > >> > > Denis > >> > >> > > > >> > >> > > > >> > >> > > On Fri, Sep 13, 2019 at 1:55 AM Denis Garus < > garus@gmail.com > >> > > >> > >> wrote: > >> > >> > > > >> > >> > > > Hello, Igniters! > >> > >> > > > > >> > >> > > > I ran two times the Examples suite [1] on the master branch > >> today > >> > >> and > >> > >> > get > >> > >> > > > different results. It looks like some tests become unstable > >> after > >> > >> > merging > >> > >> > > > of task IGNITE-12148 [2]. I think tests result depend on an > >> Agent > >> > >> that > >> > >> > > > executes suite. > >> > >> > > > > >> > >> > > > 1. > >> > >> > > > > >> > >> > > > > >> > >> > > > >> > >> > > >> > >> > >> > > >> > https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_Examples?branch=%3Cdefault%3E > >> > >> > > > > >> > >> > > > 2. > >> > >> > > > > >> > >> > > > > >> > >> > > > >> > >> > > >> > >> > >> > > >> > https://github.com/apache/ignite/commit/cd5e39b783bc499837b569c2cf974a8fe308bcf2 > >> > >> > > > > >> > >> > > > >> > >> > > >> > >> > >> > > > >> > > >> > > >
Re: After IGNITE-12148 the Examples suite has unstable tests
Alexey, about agents was my assumption, and it looks like wrong. I didn't dive so deep. пт, 13 сент. 2019 г. в 20:54, Alexey Zinoviev : > Could you help me recognize the difference between agents to exactly > reproduce the issue and be sure that I fix it? > > > пт, 13 сент. 2019 г. в 20:51, Alexey Zinoviev : > > > Thank you, I'll try to fix it, ticket is here > > https://issues.apache.org/jira/browse/IGNITE-12168 > > > > пт, 13 сент. 2019 г. в 20:41, Denis Garus : > > > >> Alexey, sure. > >> My first build today is [1], and the last build is [2]. > >> 28 tests became flaky. > >> > >> 1. > >> > >> > https://ci.ignite.apache.org/viewLog.html?buildId=4594295=IgniteTests24Java8_Examples > >> 2. > >> > >> > https://ci.ignite.apache.org/viewLog.html?buildId=4595102=IgniteTests24Java8_Examples > >> > >> пт, 13 сент. 2019 г. в 20:25, Alexey Zinoviev : > >> > >> > Dear @ Denis Garus > >> > > >> > Could you please what kind of tests became unstable? > >> > Because I have no troubles with examples (and run them of course > before > >> > merging) and many builds have no troubles too > >> > > >> > Look at > >> > > >> > > >> > https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_Examples?branch=%3Cdefault%3E > >> > - > >> > it's broken on Scala component a few weeks ago > >> > > >> > Could you please send me exact link on the builds that were ran by > >> yourself > >> > today > >> > > >> > But yes, the IGNITE-12148 could influence on many examples in ML > module > >> > because a few of resources were moved. > >> > I will try to fix today or tomorrow (but I need more information to > >> > reproduce the situation) > >> > > >> > > >> > пт, 13 сент. 2019 г. в 19:52, Denis Magda : > >> > > >> > > Alex Zinoviev, could you please double-check and confirm if > >> IGNITE-12148 > >> > > affects the test suite? > >> > > > >> > > - > >> > > Denis > >> > > > >> > > > >> > > On Fri, Sep 13, 2019 at 1:55 AM Denis Garus > >> wrote: > >> > > > >> > > > Hello, Igniters! > >> > > > > >> > > > I ran two times the Examples suite [1] on the master branch today > >> and > >> > get > >> > > > different results. It looks like some tests become unstable after > >> > merging > >> > > > of task IGNITE-12148 [2]. I think tests result depend on an Agent > >> that > >> > > > executes suite. > >> > > > > >> > > > 1. > >> > > > > >> > > > > >> > > > >> > > >> > https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_Examples?branch=%3Cdefault%3E > >> > > > > >> > > > 2. > >> > > > > >> > > > > >> > > > >> > > >> > https://github.com/apache/ignite/commit/cd5e39b783bc499837b569c2cf974a8fe308bcf2 > >> > > > > >> > > > >> > > >> > > >
Re: After IGNITE-12148 the Examples suite has unstable tests
Alexey, sure. My first build today is [1], and the last build is [2]. 28 tests became flaky. 1. https://ci.ignite.apache.org/viewLog.html?buildId=4594295=IgniteTests24Java8_Examples 2. https://ci.ignite.apache.org/viewLog.html?buildId=4595102=IgniteTests24Java8_Examples пт, 13 сент. 2019 г. в 20:25, Alexey Zinoviev : > Dear @ Denis Garus > > Could you please what kind of tests became unstable? > Because I have no troubles with examples (and run them of course before > merging) and many builds have no troubles too > > Look at > > https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_Examples?branch=%3Cdefault%3E > - > it's broken on Scala component a few weeks ago > > Could you please send me exact link on the builds that were ran by yourself > today > > But yes, the IGNITE-12148 could influence on many examples in ML module > because a few of resources were moved. > I will try to fix today or tomorrow (but I need more information to > reproduce the situation) > > > пт, 13 сент. 2019 г. в 19:52, Denis Magda : > > > Alex Zinoviev, could you please double-check and confirm if IGNITE-12148 > > affects the test suite? > > > > - > > Denis > > > > > > On Fri, Sep 13, 2019 at 1:55 AM Denis Garus wrote: > > > > > Hello, Igniters! > > > > > > I ran two times the Examples suite [1] on the master branch today and > get > > > different results. It looks like some tests become unstable after > merging > > > of task IGNITE-12148 [2]. I think tests result depend on an Agent that > > > executes suite. > > > > > > 1. > > > > > > > > > https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_Examples?branch=%3Cdefault%3E > > > > > > 2. > > > > > > > > > https://github.com/apache/ignite/commit/cd5e39b783bc499837b569c2cf974a8fe308bcf2 > > > > > >
After IGNITE-12148 the Examples suite has unstable tests
Hello, Igniters! I ran two times the Examples suite [1] on the master branch today and get different results. It looks like some tests become unstable after merging of task IGNITE-12148 [2]. I think tests result depend on an Agent that executes suite. 1. https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_Examples?branch=%3Cdefault%3E 2. https://github.com/apache/ignite/commit/cd5e39b783bc499837b569c2cf974a8fe308bcf2
Re: [DISCUSSION] REST requests explicit authorization.
Hello, Mikhail! Why do we need to avoid tough mapping GridRestCommand -> SecurityPermission? Maybe it would be more transparent if we add to the GridRestCommand a field that will contain SecurityPermission for this command? ср, 11 сент. 2019 г. в 17:34, Mikhail Petrov : > Igniters, > > I would like to suggest expanding the IgniteSecurity interface with a > method for REST requests explicit authorization (e.g. public void > authorize(GridRestRequest req) throws SecurityException;). > > Currently, REST request authorization starts in > GridRestProcessor#authorize(GridRestRequest) where GridRestCommand is > converted to SecurityPermission and then passed to > IgniteSecurity#authorize(String, SecurityPermission) for final > authorization. > > I propose to allow GridSecurityProcessor to make an authorization > decision on its own by giving it GridRestRequest. > > This approach can help to avoid tough mapping GridRestCommand -> > SecurityPermission and achieve much more flexibility in tweaking REST > request authorization. > > I will appreciate your feedback on this proposal. > >
Re: SecurityTestSuite as a separate test suite at TC
Excuse me! I was wrong. I try to find that parameter on Step 4: Run test suite. One more time, thank you! пт, 9 авг. 2019 г. в 14:05, Petr Ivanov : > Why do you think I did not use it? > > > On 9 Aug 2019, at 13:25, Denis Garus wrote: > > Great! > Could you please add surefire-fork-count-1 to additional Maven command > line parameters? > It's crucial. > > Thank you! > > пт, 9 авг. 2019 г. в 12:42, Petr Ivanov : > >> Done [1] >> >> >> [1] https://ci.ignite.apache.org/viewLog.html?buildId=4482200 >> >> On 9 Aug 2019, at 12:02, Denis Garus wrote: >> >> Sure! I created the task [1]. >> >> Thank you! >> >> 1. https://issues.apache.org/jira/browse/IGNITE-12055 >> >> пт, 9 авг. 2019 г. в 11:38, Petr Ivanov : >> >>> Hi, Denis! >>> >>> >>> Could file a ticket with description, please? >>> >>> On 9 Aug 2019, at 11:35, Denis Garus wrote: >>> >>> Thanks all for the feedback! >>> >>> I think no one is against of proposal. >>> >>> Petr, could you please assist with wit separation of SecurityTestSuite? >>> >>> чт, 8 авг. 2019 г. в 14:43, Denis Garus : >>> >>>> Hello, Ivan! >>>> >>>> >> Could you please provide more details why do we need to run these >>>> tests in forked JVM? >>>> >>>> Surefite documentation [1] says: >>>> If forkCount=0, it's impossible to use the system class loader or a >>>> plain old Java classpath; we have to use an isolated class loader. >>>> >>>> When using isolated class loader will cause compiler error: >>>> package org.apache.ignite.lang does not exist >>>> >>>> We cannot compile the TestIgniteCallable class. >>>> >>>> 1. >>>> https://maven.apache.org/surefire/maven-surefire-plugin/examples/class-loading.html >>>> >>>> чт, 8 авг. 2019 г. в 09:44, Павлухин Иван : >>>> >>>>> Denis, >>>>> >>>>> Could you please provide more details why do we need to run these >>>>> tests in forked JVM? >>>>> >>>>> Still, having separate security suite on TC sounds not bad. >>>>> >>>>> ср, 7 авг. 2019 г. в 09:35, Vyacheslav Daradur : >>>>> > >>>>> > Hi Denis. >>>>> > >>>>> > I think it is fine to extract security tests in a separate build >>>>> plan on TC. >>>>> > >>>>> > BTW, if you are going to write a lot of Sandbox's tests pay attention >>>>> > to 'extdata' module and an approach of P2P tests >>>>> > (IgniteP2PSelfTestSuite) - this may help you to avoid Maven's >>>>> > classloading issues. >>>>> > >>>>> > On Tue, Aug 6, 2019 at 3:25 PM Denis Garus >>>>> wrote: >>>>> > > >>>>> > > Hello Igniters! >>>>> > > >>>>> > > I made the test DoPrivelegedOnRemoteNodeTest[1] >>>>> (SecurityTestSuite) for the >>>>> > > task "Sandbox for user-defined code" [2] >>>>> > > that uses p2p deploy like the test >>>>> > > ServiceHotRedeploymentViaDeploymentSpiTest [3] from >>>>> > > IgniteServiceGridTestSuite. >>>>> > > That test requires additional Maven command line parameter -P >>>>> > > surefire-fork-count-1. >>>>> > > The suite Basic 1 contains the SecurityTestSuite and many other >>>>> test suites >>>>> > > at TeamCity that do not need that additional Maven parameter. >>>>> > > I suggest extracting SecurityTestSuite as a separate test suite to >>>>> define >>>>> > > additional Maven command line parameter for it. >>>>> > > >>>>> > > WDYT? >>>>> > > >>>>> > > >>>>> > > 1. https://github.com/apache/ignite/pull/6707 >>>>> > > 2. https://issues.apache.org/jira/browse/IGNITE-11410 >>>>> > > 3. >>>>> > > >>>>> https://github.com/apache/ignite/blob/master/modules/core/src/test/java/org/apache/ignite/internal/processors/service/ServiceHotRedeploymentViaDeploymentSpiTest.java >>>>> > >>>>> > >>>>> > >>>>> > -- >>>>> > Best Regards, Vyacheslav D. >>>>> >>>>> >>>>> >>>>> -- >>>>> Best regards, >>>>> Ivan Pavlukhin >>>>> >>>> >>> >> >
Re: SecurityTestSuite as a separate test suite at TC
Great! Could you please add surefire-fork-count-1 to additional Maven command line parameters? It's crucial. Thank you! пт, 9 авг. 2019 г. в 12:42, Petr Ivanov : > Done [1] > > > [1] https://ci.ignite.apache.org/viewLog.html?buildId=4482200 > > On 9 Aug 2019, at 12:02, Denis Garus wrote: > > Sure! I created the task [1]. > > Thank you! > > 1. https://issues.apache.org/jira/browse/IGNITE-12055 > > пт, 9 авг. 2019 г. в 11:38, Petr Ivanov : > >> Hi, Denis! >> >> >> Could file a ticket with description, please? >> >> On 9 Aug 2019, at 11:35, Denis Garus wrote: >> >> Thanks all for the feedback! >> >> I think no one is against of proposal. >> >> Petr, could you please assist with wit separation of SecurityTestSuite? >> >> чт, 8 авг. 2019 г. в 14:43, Denis Garus : >> >>> Hello, Ivan! >>> >>> >> Could you please provide more details why do we need to run these >>> tests in forked JVM? >>> >>> Surefite documentation [1] says: >>> If forkCount=0, it's impossible to use the system class loader or a >>> plain old Java classpath; we have to use an isolated class loader. >>> >>> When using isolated class loader will cause compiler error: >>> package org.apache.ignite.lang does not exist >>> >>> We cannot compile the TestIgniteCallable class. >>> >>> 1. >>> https://maven.apache.org/surefire/maven-surefire-plugin/examples/class-loading.html >>> >>> чт, 8 авг. 2019 г. в 09:44, Павлухин Иван : >>> >>>> Denis, >>>> >>>> Could you please provide more details why do we need to run these >>>> tests in forked JVM? >>>> >>>> Still, having separate security suite on TC sounds not bad. >>>> >>>> ср, 7 авг. 2019 г. в 09:35, Vyacheslav Daradur : >>>> > >>>> > Hi Denis. >>>> > >>>> > I think it is fine to extract security tests in a separate build plan >>>> on TC. >>>> > >>>> > BTW, if you are going to write a lot of Sandbox's tests pay attention >>>> > to 'extdata' module and an approach of P2P tests >>>> > (IgniteP2PSelfTestSuite) - this may help you to avoid Maven's >>>> > classloading issues. >>>> > >>>> > On Tue, Aug 6, 2019 at 3:25 PM Denis Garus >>>> wrote: >>>> > > >>>> > > Hello Igniters! >>>> > > >>>> > > I made the test DoPrivelegedOnRemoteNodeTest[1] (SecurityTestSuite) >>>> for the >>>> > > task "Sandbox for user-defined code" [2] >>>> > > that uses p2p deploy like the test >>>> > > ServiceHotRedeploymentViaDeploymentSpiTest [3] from >>>> > > IgniteServiceGridTestSuite. >>>> > > That test requires additional Maven command line parameter -P >>>> > > surefire-fork-count-1. >>>> > > The suite Basic 1 contains the SecurityTestSuite and many other >>>> test suites >>>> > > at TeamCity that do not need that additional Maven parameter. >>>> > > I suggest extracting SecurityTestSuite as a separate test suite to >>>> define >>>> > > additional Maven command line parameter for it. >>>> > > >>>> > > WDYT? >>>> > > >>>> > > >>>> > > 1. https://github.com/apache/ignite/pull/6707 >>>> > > 2. https://issues.apache.org/jira/browse/IGNITE-11410 >>>> > > 3. >>>> > > >>>> https://github.com/apache/ignite/blob/master/modules/core/src/test/java/org/apache/ignite/internal/processors/service/ServiceHotRedeploymentViaDeploymentSpiTest.java >>>> > >>>> > >>>> > >>>> > -- >>>> > Best Regards, Vyacheslav D. >>>> >>>> >>>> >>>> -- >>>> Best regards, >>>> Ivan Pavlukhin >>>> >>> >> >
Re: SecurityTestSuite as a separate test suite at TC
Sure! I created the task [1]. Thank you! 1. https://issues.apache.org/jira/browse/IGNITE-12055 пт, 9 авг. 2019 г. в 11:38, Petr Ivanov : > Hi, Denis! > > > Could file a ticket with description, please? > > On 9 Aug 2019, at 11:35, Denis Garus wrote: > > Thanks all for the feedback! > > I think no one is against of proposal. > > Petr, could you please assist with wit separation of SecurityTestSuite? > > чт, 8 авг. 2019 г. в 14:43, Denis Garus : > >> Hello, Ivan! >> >> >> Could you please provide more details why do we need to run these >> tests in forked JVM? >> >> Surefite documentation [1] says: >> If forkCount=0, it's impossible to use the system class loader or a plain >> old Java classpath; we have to use an isolated class loader. >> >> When using isolated class loader will cause compiler error: >> package org.apache.ignite.lang does not exist >> >> We cannot compile the TestIgniteCallable class. >> >> 1. >> https://maven.apache.org/surefire/maven-surefire-plugin/examples/class-loading.html >> >> чт, 8 авг. 2019 г. в 09:44, Павлухин Иван : >> >>> Denis, >>> >>> Could you please provide more details why do we need to run these >>> tests in forked JVM? >>> >>> Still, having separate security suite on TC sounds not bad. >>> >>> ср, 7 авг. 2019 г. в 09:35, Vyacheslav Daradur : >>> > >>> > Hi Denis. >>> > >>> > I think it is fine to extract security tests in a separate build plan >>> on TC. >>> > >>> > BTW, if you are going to write a lot of Sandbox's tests pay attention >>> > to 'extdata' module and an approach of P2P tests >>> > (IgniteP2PSelfTestSuite) - this may help you to avoid Maven's >>> > classloading issues. >>> > >>> > On Tue, Aug 6, 2019 at 3:25 PM Denis Garus >>> wrote: >>> > > >>> > > Hello Igniters! >>> > > >>> > > I made the test DoPrivelegedOnRemoteNodeTest[1] (SecurityTestSuite) >>> for the >>> > > task "Sandbox for user-defined code" [2] >>> > > that uses p2p deploy like the test >>> > > ServiceHotRedeploymentViaDeploymentSpiTest [3] from >>> > > IgniteServiceGridTestSuite. >>> > > That test requires additional Maven command line parameter -P >>> > > surefire-fork-count-1. >>> > > The suite Basic 1 contains the SecurityTestSuite and many other test >>> suites >>> > > at TeamCity that do not need that additional Maven parameter. >>> > > I suggest extracting SecurityTestSuite as a separate test suite to >>> define >>> > > additional Maven command line parameter for it. >>> > > >>> > > WDYT? >>> > > >>> > > >>> > > 1. https://github.com/apache/ignite/pull/6707 >>> > > 2. https://issues.apache.org/jira/browse/IGNITE-11410 >>> > > 3. >>> > > >>> https://github.com/apache/ignite/blob/master/modules/core/src/test/java/org/apache/ignite/internal/processors/service/ServiceHotRedeploymentViaDeploymentSpiTest.java >>> > >>> > >>> > >>> > -- >>> > Best Regards, Vyacheslav D. >>> >>> >>> >>> -- >>> Best regards, >>> Ivan Pavlukhin >>> >> >
[jira] [Created] (IGNITE-12055) SecurityTestSuite as a separate test suite at TC
Denis Garus created IGNITE-12055: Summary: SecurityTestSuite as a separate test suite at TC Key: IGNITE-12055 URL: https://issues.apache.org/jira/browse/IGNITE-12055 Project: Ignite Issue Type: Task Components: security Reporter: Denis Garus Need exclude SecurityTestSuite from IgniteBasicTestSuite (Basic 1). Create suite Security with SecurityTestSuite at TC. Config is the same as Basic 1 except additional Maven command line parameters that should be: -P all-java,all-other,scala-2.10,tensorflow,scala,scala-test,surefire-fork-count-1 -pl %MAVEN_MODULES% -am -Dmaven.test.failure.ignore=true -DfailIfNoTests=false -Dtest=%TEST_SUITE% -Dmaven.javadoc.skip=true %MAVEN_OPTS% Include SecurityTestSuite to Run::All, Run::All (Nightly), Run::Basic Tests at TC. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
Re: SecurityTestSuite as a separate test suite at TC
Thanks all for the feedback! I think no one is against of proposal. Petr, could you please assist with wit separation of SecurityTestSuite? чт, 8 авг. 2019 г. в 14:43, Denis Garus : > Hello, Ivan! > > >> Could you please provide more details why do we need to run these tests > in forked JVM? > > Surefite documentation [1] says: > If forkCount=0, it's impossible to use the system class loader or a plain > old Java classpath; we have to use an isolated class loader. > > When using isolated class loader will cause compiler error: > package org.apache.ignite.lang does not exist > > We cannot compile the TestIgniteCallable class. > > 1. > https://maven.apache.org/surefire/maven-surefire-plugin/examples/class-loading.html > > чт, 8 авг. 2019 г. в 09:44, Павлухин Иван : > >> Denis, >> >> Could you please provide more details why do we need to run these >> tests in forked JVM? >> >> Still, having separate security suite on TC sounds not bad. >> >> ср, 7 авг. 2019 г. в 09:35, Vyacheslav Daradur : >> > >> > Hi Denis. >> > >> > I think it is fine to extract security tests in a separate build plan >> on TC. >> > >> > BTW, if you are going to write a lot of Sandbox's tests pay attention >> > to 'extdata' module and an approach of P2P tests >> > (IgniteP2PSelfTestSuite) - this may help you to avoid Maven's >> > classloading issues. >> > >> > On Tue, Aug 6, 2019 at 3:25 PM Denis Garus wrote: >> > > >> > > Hello Igniters! >> > > >> > > I made the test DoPrivelegedOnRemoteNodeTest[1] (SecurityTestSuite) >> for the >> > > task "Sandbox for user-defined code" [2] >> > > that uses p2p deploy like the test >> > > ServiceHotRedeploymentViaDeploymentSpiTest [3] from >> > > IgniteServiceGridTestSuite. >> > > That test requires additional Maven command line parameter -P >> > > surefire-fork-count-1. >> > > The suite Basic 1 contains the SecurityTestSuite and many other test >> suites >> > > at TeamCity that do not need that additional Maven parameter. >> > > I suggest extracting SecurityTestSuite as a separate test suite to >> define >> > > additional Maven command line parameter for it. >> > > >> > > WDYT? >> > > >> > > >> > > 1. https://github.com/apache/ignite/pull/6707 >> > > 2. https://issues.apache.org/jira/browse/IGNITE-11410 >> > > 3. >> > > >> https://github.com/apache/ignite/blob/master/modules/core/src/test/java/org/apache/ignite/internal/processors/service/ServiceHotRedeploymentViaDeploymentSpiTest.java >> > >> > >> > >> > -- >> > Best Regards, Vyacheslav D. >> >> >> >> -- >> Best regards, >> Ivan Pavlukhin >> >
Re: Coding guidelines. Useless JavaDoc comments.
Thank you, Maxim.
>>On what we are trying to save by making Javadoc optional?
Space and time.
I doubt that we need a verbal description of what private method make.
Why we need it if we just can read the code?
Bright examples:
/**
* @param helper Helper to attach to kernal context.
*/
private void addHelper(Object helper) {
ctx.addHelper(helper);
}
/**
* Gets "on" or "off" string for given boolean value.
*
* @param b Boolean value to convert.
* @return Result string.
*/
private String onOff(boolean b) {
return b ? "on" : "off";
}
You have to support both code of method and their JavaDoc description, but
it doesn't get any sake.
>>Let's just help them to read the source code.
But at the same time, public method IgniteKernal#start doesn't have any
description except enumeration of attributes with apparent notes.
Maybe to give it a verbal description needs one or two pages of the screen.
Does it make sense?
чт, 8 авг. 2019 г. в 15:41, Maxim Muzafarov :
> Igniters,
>
> I'm -1 with making Javadoc optional (except tests).
>
> Here is my patch [1] with PR [2] which fixes all the Javadoc comments
> for the IgniteKernal class mentioned above. Please, take a look. The
> review is very appreciated.
>
> On what we are trying to save by making Javadoc optional? In my humble
> opinion - Javadoc comments are mostly concern users who debug the
> Ignite when they have faced with some unhandled exception or related
> to the community members with an average involvement (produces a few
> small patches during the year) but not to the experienced one. Let's
> just help them to read the source code.
>
> [1] https://issues.apache.org/jira/browse/IGNITE-12051
> [2] https://github.com/apache/ignite/pull/6760
>
>
>
> On Wed, 7 Aug 2019 at 13:54, Andrey Kuznetsov wrote:
> >
> > +1 for making javadoc comments optional.
> >
> > - Empty and tautological comments are kind of garbage that reduce
> > readability.
> > - It's better to leave the entity undocumented, than write
> > unexpressive/misleading comment.
> > - Even classes may not require javadocs, e.g. simple DTOs.
> >
> > ср, 7 авг. 2019 г., 13:39 Denis Garus :
> >
> > > Thx for feedback!
> > >
> > > >> we have to write proper javadoc for all production classes,
> including
> > > internal.
> > >
> > > Nikolay, I cannot agree with it.
> > >
> > > What should be the best comment for the next fields?
> > > /** */
> > > private static final long serialVersionUID = 0L;
> > > or
> > > /** */
> > > @LoggerResource
> > > private IgniteLogger log;
> > >
> > > There are more than 8000 lines of /** */ only at the ignite-core
> module (do
> > > not include tests).
> > >
> > > Any comments will be redundant and just noise. Obvious comment learns
> > > readers skip all comments.
> > >
> > >
> > > >> identical distance/padding/margin between fields in a class - is
> really
> > > cool
> > >
> > > Vyacheslav, but we have a blank line between fields. Why is one blank
> line
> > > not enough?
> > >
> > > ср, 7 авг. 2019 г. в 12:58, Павлухин Иван :
> > >
> > > > Hi,
> > > >
> > > > Denis, thank you for starting this discussion!
> > > >
> > > > My opinion here is that having a good javadoc for every class and
> > > > method is not feasible in the real world. I am quite curious to see a
> > > > non-trivial project which follows it. Also, all comments and javadocs
> > > > are prone to become misleading when code changes (human factor). In
> my
> > > > experience good method and variable names and clear code organization
> > > > often are more helpful than javadocs.
> > > >
> > > > ср, 7 авг. 2019 г. в 12:49, Vyacheslav Daradur >:
> > > > >
> > > > > I agree that useless comments look weird in the codebase.
> > > > >
> > > > > But, identical distance/padding/margin between fields in a class -
> is
> > > > > really cool, and helps read the class very fast.
> > > > >
> > > > > On Wed, Aug 7, 2019 at 12:26 PM Nikolay Izhikov <
> nizhi...@apache.org>
> > > > wrote:
> > > > > >
> > > > > > Hello, Denis.
> > > > > >
> > > > > > Thanks for starting this discussion.
> > > > > >
> > > > > > I think we have to write proper javadoc for all produc
Re: SecurityTestSuite as a separate test suite at TC
Hello, Ivan! >> Could you please provide more details why do we need to run these tests in forked JVM? Surefite documentation [1] says: If forkCount=0, it's impossible to use the system class loader or a plain old Java classpath; we have to use an isolated class loader. When using isolated class loader will cause compiler error: package org.apache.ignite.lang does not exist We cannot compile the TestIgniteCallable class. 1. https://maven.apache.org/surefire/maven-surefire-plugin/examples/class-loading.html чт, 8 авг. 2019 г. в 09:44, Павлухин Иван : > Denis, > > Could you please provide more details why do we need to run these > tests in forked JVM? > > Still, having separate security suite on TC sounds not bad. > > ср, 7 авг. 2019 г. в 09:35, Vyacheslav Daradur : > > > > Hi Denis. > > > > I think it is fine to extract security tests in a separate build plan on > TC. > > > > BTW, if you are going to write a lot of Sandbox's tests pay attention > > to 'extdata' module and an approach of P2P tests > > (IgniteP2PSelfTestSuite) - this may help you to avoid Maven's > > classloading issues. > > > > On Tue, Aug 6, 2019 at 3:25 PM Denis Garus wrote: > > > > > > Hello Igniters! > > > > > > I made the test DoPrivelegedOnRemoteNodeTest[1] (SecurityTestSuite) > for the > > > task "Sandbox for user-defined code" [2] > > > that uses p2p deploy like the test > > > ServiceHotRedeploymentViaDeploymentSpiTest [3] from > > > IgniteServiceGridTestSuite. > > > That test requires additional Maven command line parameter -P > > > surefire-fork-count-1. > > > The suite Basic 1 contains the SecurityTestSuite and many other test > suites > > > at TeamCity that do not need that additional Maven parameter. > > > I suggest extracting SecurityTestSuite as a separate test suite to > define > > > additional Maven command line parameter for it. > > > > > > WDYT? > > > > > > > > > 1. https://github.com/apache/ignite/pull/6707 > > > 2. https://issues.apache.org/jira/browse/IGNITE-11410 > > > 3. > > > > https://github.com/apache/ignite/blob/master/modules/core/src/test/java/org/apache/ignite/internal/processors/service/ServiceHotRedeploymentViaDeploymentSpiTest.java > > > > > > > > -- > > Best Regards, Vyacheslav D. > > > > -- > Best regards, > Ivan Pavlukhin >
Re: Coding guidelines. Useless JavaDoc comments.
Thx for feedback! >> we have to write proper javadoc for all production classes, including internal. Nikolay, I cannot agree with it. What should be the best comment for the next fields? /** */ private static final long serialVersionUID = 0L; or /** */ @LoggerResource private IgniteLogger log; There are more than 8000 lines of /** */ only at the ignite-core module (do not include tests). Any comments will be redundant and just noise. Obvious comment learns readers skip all comments. >> identical distance/padding/margin between fields in a class - is really cool Vyacheslav, but we have a blank line between fields. Why is one blank line not enough? ср, 7 авг. 2019 г. в 12:58, Павлухин Иван : > Hi, > > Denis, thank you for starting this discussion! > > My opinion here is that having a good javadoc for every class and > method is not feasible in the real world. I am quite curious to see a > non-trivial project which follows it. Also, all comments and javadocs > are prone to become misleading when code changes (human factor). In my > experience good method and variable names and clear code organization > often are more helpful than javadocs. > > ср, 7 авг. 2019 г. в 12:49, Vyacheslav Daradur : > > > > I agree that useless comments look weird in the codebase. > > > > But, identical distance/padding/margin between fields in a class - is > > really cool, and helps read the class very fast. > > > > On Wed, Aug 7, 2019 at 12:26 PM Nikolay Izhikov > wrote: > > > > > > Hello, Denis. > > > > > > Thanks for starting this discussion. > > > > > > I think we have to write proper javadoc for all production classes, > including internal. > > > We should fix useless javadoc you provide. > > > We should not accept patches without good javadocs. > > > > > > As for the tests, seems, we can make javadoc optional. > > > > > > What do you think? > > > > > > > > > В Ср, 07/08/2019 в 11:41 +0300, Denis Garus пишет: > > > > Igniters! > > > > > > > > I think it's time to change coding guidelines in part of JavaDoc > comments > > > > [1]: > > > > > > Every method, field or initializer public, private or protected > in > > > > > > > > top-level, > > > > > > inner or anonymous type should have at least minimal Javadoc > comments > > > > > > > > including > > > > > > description and description of parameters using @param, @return > and > > > > > > > > @throws Javadoc tags, > > > > > > where applicable. > > > > > > > > Let's look at JavaDoc comments in the IgniteKernal class: > > > > > > > > Why? > > > > > > > > /** */ - 15 matches. > > > > > > > > What can you get new from these comments? > > > > > > > > /** Periodic starvation check interval. */ > > > > private static final long PERIODIC_STARVATION_CHECK_FREQ = 1000 * 30; > > > > > > > > /** Long jvm pause detector. */ > > > > private LongJVMPauseDetector longJVMPauseDetector; > > > > > > > > /** Scheduler. */ > > > > private IgniteScheduler scheduler; > > > > > > > > /** Stop guard. */ > > > > private final AtomicBoolean stopGuard = new AtomicBoolean(); > > > > > > > > /** > > > > * @param cfg Configuration to use. > > > > * @param utilityCachePool Utility cache pool. > > > > * @param execSvc Executor service. > > > > * @param sysExecSvc System executor service. > > > > * @param stripedExecSvc Striped executor. > > > > * @param p2pExecSvc P2P executor service. > > > > * @param mgmtExecSvc Management executor service. > > > > * @param igfsExecSvc IGFS executor service. > > > > * @param dataStreamExecSvc data stream executor service. > > > > * @param restExecSvc Reset executor service. > > > > * @param affExecSvc Affinity executor service. > > > > * @param idxExecSvc Indexing executor service. > > > > * @param callbackExecSvc Callback executor service. > > > > * @param qryExecSvc Query executor service. > > > > * @param schemaExecSvc Schema executor service. > > > > * @param customExecSvcs Custom named executors. > > > > * @param errHnd Error handler to use for notification about > startu
Coding guidelines. Useless JavaDoc comments.
Igniters! I think it's time to change coding guidelines in part of JavaDoc comments [1]: >> Every method, field or initializer public, private or protected in top-level, >> inner or anonymous type should have at least minimal Javadoc comments including >> description and description of parameters using @param, @return and @throws Javadoc tags, >> where applicable. Let's look at JavaDoc comments in the IgniteKernal class: Why? /** */ - 15 matches. What can you get new from these comments? /** Periodic starvation check interval. */ private static final long PERIODIC_STARVATION_CHECK_FREQ = 1000 * 30; /** Long jvm pause detector. */ private LongJVMPauseDetector longJVMPauseDetector; /** Scheduler. */ private IgniteScheduler scheduler; /** Stop guard. */ private final AtomicBoolean stopGuard = new AtomicBoolean(); /** * @param cfg Configuration to use. * @param utilityCachePool Utility cache pool. * @param execSvc Executor service. * @param sysExecSvc System executor service. * @param stripedExecSvc Striped executor. * @param p2pExecSvc P2P executor service. * @param mgmtExecSvc Management executor service. * @param igfsExecSvc IGFS executor service. * @param dataStreamExecSvc data stream executor service. * @param restExecSvc Reset executor service. * @param affExecSvc Affinity executor service. * @param idxExecSvc Indexing executor service. * @param callbackExecSvc Callback executor service. * @param qryExecSvc Query executor service. * @param schemaExecSvc Schema executor service. * @param customExecSvcs Custom named executors. * @param errHnd Error handler to use for notification about startup problems. * @param workerRegistry Worker registry. * @param hnd Default uncaught exception handler used by thread pools. * @throws IgniteCheckedException Thrown in case of any errors. */ public void start( final IgniteConfiguration cfg, ExecutorService utilityCachePool, final ExecutorService execSvc, final ExecutorService svcExecSvc, final ExecutorService sysExecSvc, final StripedExecutor stripedExecSvc, ExecutorService p2pExecSvc, ExecutorService mgmtExecSvc, ExecutorService igfsExecSvc, StripedExecutor dataStreamExecSvc, ExecutorService restExecSvc, ExecutorService affExecSvc, @Nullable ExecutorService idxExecSvc, IgniteStripedThreadPoolExecutor callbackExecSvc, ExecutorService qryExecSvc, ExecutorService schemaExecSvc, @Nullable final Map customExecSvcs, GridAbsClosure errHnd, WorkersRegistry workerRegistry, Thread.UncaughtExceptionHandler hnd, TimeBag startTimer ) These comments look ugly and useless, and that is the main class of core. Why do they need us? Let us change the coding guidelines in part of JavaDoc comments: Every method public API should have at least minimal Javadoc comments, including description and description of parameters using @param, @return, and @throws Javadoc tags, where applicable. For internal API, JavaDoc comments should be optional. It's up to a contributor or reviewer. What are you think? 1. https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-JavadocComments
SecurityTestSuite as a separate test suite at TC
Hello Igniters! I made the test DoPrivelegedOnRemoteNodeTest[1] (SecurityTestSuite) for the task "Sandbox for user-defined code" [2] that uses p2p deploy like the test ServiceHotRedeploymentViaDeploymentSpiTest [3] from IgniteServiceGridTestSuite. That test requires additional Maven command line parameter -P surefire-fork-count-1. The suite Basic 1 contains the SecurityTestSuite and many other test suites at TeamCity that do not need that additional Maven parameter. I suggest extracting SecurityTestSuite as a separate test suite to define additional Maven command line parameter for it. WDYT? 1. https://github.com/apache/ignite/pull/6707 2. https://issues.apache.org/jira/browse/IGNITE-11410 3. https://github.com/apache/ignite/blob/master/modules/core/src/test/java/org/apache/ignite/internal/processors/service/ServiceHotRedeploymentViaDeploymentSpiTest.java
Re: Improvements for new security approach.
Hello, Maksim! Thanks for your analysis! I have a few questions about your proposals. GridRestProcessor. AFAIK, when GridRestProcessor handle client request (GridRestProcessor#handleRequest) it process authentication (GridRestProcessor#authenticate) and then authorization of request (GridRestProcessor#authorize) inside client context. Can you give additional info about issues with GridRestProcessor from 3 and 4? Maybe you have a reproducer for the problem? NoOpIgniteSecurityProcessor. I think the case that you describe in 5 is not a bug. All nodes (client and server) must have security enabled or disabled. I can't imagine the case when it is not. ATTR_SECURITY_SUBJECT. I don't think that compatibility is needed here. If you will use node with propagation security context to remote node and older nodes you can get subtle errors. чт, 18 июл. 2019 г. в 11:12, Maksim Stepachev : > Hi, Ivan. > > Yes, I have. > https://issues.apache.org/jira/browse/IGNITE-11992 > > I'm waiting for a visa. > > > чт, 18 июл. 2019 г. в 11:09, Ivan Rakov : > >> Hello Max, >> >> Thanks for your analysis! >> >> Have you created a JIRA issue for discovered defects? >> >> Best Regards, >> Ivan Rakov >> >> On 17.07.2019 17:08, Maksim Stepachev wrote: >> > Hello, Igniters. >> > >> > The main idea of the new security is propagation security context >> to >> > other nodes and does action with initial permission. The solution looks >> > fine but has imperfections. >> > >> > 1. ZookeaperDiscoveryImpl doesn't implement security into itself. >> >As a result: Caused by: class >> org.apache.ignite.spi.IgniteSpiException: >> > Security context isn't certain. >> > 2. The visor tasks lost permission. >> > The method VisorQueryUtils#scheduleQueryStart makes a new thread and >> loses >> > context. >> > 3. The GridRestProcessor does tasks outside "withContext" section. As >> > result context loses. >> > 4. The GridRestProcessor isn't client, we can't read security subject >> from >> > node attribute. >> > We should transmit secCtx for fake nodes and secSubjId for real. >> > 5. NoOpIgniteSecurityProcessor should include a disabled processor and >> > validate it too if it is not null. It is important for a client node. >> > For example: >> > Into IgniteKernal#securityProcessor method createComponent return a >> > GridSecurityProcessor. For server nodes are enabled, but for clients >> > aren't. The clients aren't able to pass validation for this reason. >> > >> > 6. ATTR_SECURITY_SUBJECT was removed. It broke compatibility. >> > >> > I going to fix it. >> > >> >
