Thanks, Rajini. It looks good to me. best, Colin
On Thu, Aug 15, 2019, at 11:37, Rajini Sivaram wrote: > Hi Colin, > > Thanks for the review. I have updated the KIP to move the interfaces for > request context and server info to the authorizer package. These are now > called AuthorizableRequestContext and AuthorizerServerInfo. Endpoint is now > a class in org.apache.kafka.common to make it reusable since we already > have multiple implementations of it. I have removed requestName from the > request context interface since authorizers can distinguish follower fetch > and consumer fetch from the operation being authorized. So 16-bit request > type should be sufficient for audit logging. Also replaced AuditFlag with > two booleans as you suggested. > > Can you take another look and see if the KIP is ready for voting? > > Thanks for all your help! > > Regards, > > Rajini > > On Wed, Aug 14, 2019 at 8:59 PM Colin McCabe <cmcc...@apache.org> wrote: > > > Hi Rajini, > > > > I think it would be good to rename KafkaRequestContext to something like > > AuthorizableRequestContext, and put it in the > > org.apache.kafka.server.authorizer namespace. If we put it in the > > org.apache.kafka.common namespace, then it's not really clear that it's > > part of the Authorizer API. Since this class is purely an interface, with > > no concrete implementation of anything, there's nothing common to really > > reuse in any case. We definitely don't want someone to accidentally add or > > remove methods because they think this is just another internal class used > > for requests. > > > > The BrokerInfo class is a nice improvement. It looks like it will be > > useful for passing in information about the context we're running in. It > > would be nice to call this class ServerInfo rather than BrokerInfo, since > > we will want to run the authorizer on controllers as well as on brokers, > > and the controller may run as a separate process post KIP-500. I also > > think that this class should be in the org.apache.kafka.server.authorizer > > namespace. Again, it is an interface, not a concrete implementation, and > > it's an interface that is very specifically for the authorizer. > > > > I agree that we probably don't have enough information preserved for > > requests currently to always know what entity made them. So we can leave > > that out for now (except in the special case of Fetch). Perhaps we can add > > this later if it's needed. > > > > I understand the intention behind AuthorizationMode (which is now called > > AuditFlag in the latest revision). But it still feels complex. What if we > > just had two booleans in Action: logSuccesses and logFailures? That seems > > to cover all the cases here. MANDATORY_AUTHORIZE = true, true. > > OPTIONAL_AUTHORIZE = true, false. FILTER = true, false. LIST_AUTHORIZED = > > false, false. Would there be anything lost versus having the enum? > > > > best, > > Colin > > > > > > On Wed, Aug 14, 2019, at 06:29, Mickael Maison wrote: > > > Hi Rajini, > > > > > > Thanks for the KIP! > > > I really like that authorize() will be able to take a batch of > > > requests, this will speed up many implementations! > > > > > > On Tue, Aug 13, 2019 at 5:57 PM Rajini Sivaram <rajinisiva...@gmail.com> > > wrote: > > > > > > > > Thanks David! I have fixed the typo. > > > > > > > > Also made a couple of changes to make the context interfaces more > > generic. > > > > KafkaRequestContext now returns the 16-bit API key as Colin suggested > > as > > > > well as the friendly name used in metrics which are useful in audit > > logs. > > > > `Authorizer#start` is now provided a generic `BrokerInfo` interface > > that > > > > gives cluster id, broker id and endpoint information. The generic > > interface > > > > can potentially be used in other broker plugins in future and provides > > > > dynamically generated configs like broker id and ports which are > > currently > > > > not available to plugins unless these configs are statically > > configured. > > > > Please let me know if there are any concerns. > > > > > > > > Regards, > > > > > > > > Rajini > > > > > > > > On Tue, Aug 13, 2019 at 4:30 PM David Jacot <dja...@confluent.io> > > wrote: > > > > > > > > > Hi Rajini, > > > > > > > > > > Thank you for the update! It looks good to me. There is a typo in the > > > > > `AuditFlag` enum: `MANDATORY_AUTHOEIZE` -> `MANDATORY_AUTHORIZE`. > > > > > > > > > > Regards, > > > > > David > > > > > > > > > > On Mon, Aug 12, 2019 at 2:54 PM Rajini Sivaram < > > rajinisiva...@gmail.com> > > > > > wrote: > > > > > > > > > > > Hi David, > > > > > > > > > > > > Thanks for reviewing the KIP! Since questions about `authorization > > mode` > > > > > > and `count` have come up multiple times, I have renamed both. > > > > > > > > > > > > 1) Renamed `count` to `resourceReferenceCount`. It is the number > > of times > > > > > > the resource being authorized is referenced within the request. > > > > > > > > > > > > 2) Renamed `AuthorizationMode` to `AuditFlag`. It is provided to > > improve > > > > > > audit logging in the authorizer. The enum values have javadoc which > > > > > > indicate how the authorization result is used in each of the modes > > to > > > > > > enable authorizers to log audit messages at the right severity > > level. > > > > > > > > > > > > Regards, > > > > > > > > > > > > Rajini > > > > > > > > > > > > On Mon, Aug 12, 2019 at 5:54 PM David Jacot <dja...@confluent.io> > > wrote: > > > > > > > > > > > > > Hi Rajini, > > > > > > > > > > > > > > Thank you for the KIP. Overall, it looks good to me. I have few > > > > > > > questions/suggestions: > > > > > > > > > > > > > > 1. It is hard to grasp what `Action#count` is for. I guess I > > understand > > > > > > > where you want to go with it but it took me a while to figure it > > out. > > > > > > > Perhaps, we could come up with a better name than `count`? > > > > > > > > > > > > > > 2. I had a hard time trying to understand the `AuthorizationMode` > > > > > > concept, > > > > > > > especially wrt. the OPTIONAL one. My understanding is that an > > ACL is > > > > > > either > > > > > > > defined or not. Could you elaborate a bit more on that? > > > > > > > > > > > > > > Thanks, > > > > > > > David > > > > > > > > > > > > > > On Fri, Aug 9, 2019 at 10:26 PM Don Bosco Durai < > > bo...@apache.org> > > > > > > wrote: > > > > > > > > > > > > > > > Hi Rajini > > > > > > > > > > > > > > > > 3.2 - This makes sense. Thanks for clarifying. > > > > > > > > > > > > > > > > Rest looks fine. Once the implementations are done, it will be > > more > > > > > > clear > > > > > > > > on the different values RequestType and Mode. > > > > > > > > > > > > > > > > Thanks > > > > > > > > > > > > > > > > Bosco > > > > > > > > > > > > > > > > > > > > > > > > On 8/9/19, 5:08 AM, "Rajini Sivaram" <rajinisiva...@gmail.com > > > > > > > > wrote: > > > > > > > > > > > > > > > > Hi Don, > > > > > > > > > > > > > > > > Thanks for the suggestions. A few responses below: > > > > > > > > > > > > > > > > 3.1 Can rename and improve docs if we keep this. Let's > > finish the > > > > > > > > discussion on Colin's suggestions regarding this first. > > > > > > > > 3.2 No, I was thinking of some requests that have an old > > way of > > > > > > > > authorizing > > > > > > > > and a new way where we have retained the old way for > > backward > > > > > > > > compatibility. One example is Cluster:Create permission to > > create > > > > > > > > topics. > > > > > > > > We have replaced this with fine-grained topic create > > access using > > > > > > > > Topic:Create > > > > > > > > for topic patterns. But we still check if user has > > Cluster:Create > > > > > > > > first. If > > > > > > > > Denied, the deny is ignored and we check Topic:Create. We > > dont > > > > > want > > > > > > > to > > > > > > > > log > > > > > > > > DENY for Cluster:Create at INFO level for this, since this > > is > > > > > not a > > > > > > > > mandatory ACL for creating topics. We will get appropriate > > logs > > > > > > from > > > > > > > > the > > > > > > > > subsequent Topic:Create in this case. > > > > > > > > 3.3 They are not quite the same. FILTER implies that user > > > > > actually > > > > > > > > requested access to and performed some operation on the > > filtered > > > > > > > > resources. > > > > > > > > LIST_AUTHORZED did not result in any actual access. User > > simply > > > > > > > wanted > > > > > > > > to > > > > > > > > know what they are allowed to access. > > > > > > > > 3.4 Request types are Produce, JoinGroup, OffsetCommit > > etc. So > > > > > that > > > > > > > is > > > > > > > > different from authorization mode, operation etc. > > > > > > > > > > > > > > > > > > > > > > > > On Thu, Aug 8, 2019 at 11:36 PM Don Bosco Durai < > > > > > bo...@apache.org> > > > > > > > > wrote: > > > > > > > > > > > > > > > > > Hi Rajini > > > > > > > > > > > > > > > > > > Thanks for clarifying. This is very helpful... > > > > > > > > > > > > > > > > > > #1 - On the Ranger side, we should be able to handle > > multiple > > > > > > > > requests at > > > > > > > > > the same time. I was just not sure how much processing > > overhead > > > > > > > will > > > > > > > > be > > > > > > > > > there on the Broker side to split and then consolidate > > the > > > > > > results. > > > > > > > > If it > > > > > > > > > is negligible, then this is the better way. It will > > make it > > > > > > future > > > > > > > > proof. > > > > > > > > > #2 - I agree, having a single "start" call makes it > > cleaner. > > > > > The > > > > > > > > > Authorization implementation will only have to do the > > > > > > > initialization > > > > > > > > only > > > > > > > > > once. > > > > > > > > > #3.1 - Thanks for the clarification. I think it makes > > sense to > > > > > > have > > > > > > > > this. > > > > > > > > > The term "Mode" might not be explicit enough. > > Essentially it > > > > > > seems > > > > > > > > you want > > > > > > > > > the Authorizer to know the Intent/Purpose of the > > authorize call > > > > > > and > > > > > > > > let the > > > > > > > > > Authorizer decide what to log as Audit event. Changing > > the > > > > > > > > class/field name > > > > > > > > > or giving more documentation will do. > > > > > > > > > #3.2 - Regarding the option "OPTIONAL". Are you thinking > > from > > > > > > > > chaining > > > > > > > > > multiple Authorizer? If so, I am not sure whether the > > Broker > > > > > > would > > > > > > > > have > > > > > > > > > enough information to make that decision. I feel the > > Authorizer > > > > > > > will > > > > > > > > be the > > > > > > > > > one who would have that knowledge. E.g. in Ranger we have > > > > > > explicit > > > > > > > > deny, > > > > > > > > > which means no matter what, the request should be denied > > for > > > > > the > > > > > > > > user/group > > > > > > > > > or condition. So if you are thinking of chaining > > Authorizers, > > > > > > then > > > > > > > > > responses should have the third state, e.g. > > "DENIED_FINAL", in > > > > > > > which > > > > > > > > case > > > > > > > > > if there is an Authorization chain, it will be stop and > > the > > > > > > request > > > > > > > > will be > > > > > > > > > denied and if it is just denied, then you might fall > > back to > > > > > next > > > > > > > > > authorizer. If we don't have chaining of Authorizing in > > mind, > > > > > > then > > > > > > > we > > > > > > > > > should reconsider OPTIONAL for now. Or clarify under > > which > > > > > > scenario > > > > > > > > > OPTIONAL will be called. > > > > > > > > > #3.3 Regarding, FILTER v/s LIST_AUTHORIZED, isn't > > > > > > LIST_AUTHORIZED a > > > > > > > > > special case for "FILTER"? > > > > > > > > > #3.4 KafkaRequestContext. requestType() v/s Action. > > > > > > > > authorizationMode. I > > > > > > > > > am not sure about the overlap or ambiguity. > > > > > > > > > #4 - Cool. This is good, it will be less stress on the > > > > > > Authorizer. > > > > > > > > Ranger > > > > > > > > > already supports the "count" concept and also has > > batching > > > > > > > > capability to > > > > > > > > > aggregate similar requests to reduce the number of audit > > logs > > > > > to > > > > > > > > write. We > > > > > > > > > should be able to pass this through. > > > > > > > > > #5 - Assuming if the object instance is going out of > > scope, we > > > > > > > > should be > > > > > > > > > fine. Not a super important ask. Ranger is already > > catching > > > > > KILL > > > > > > > > signal for > > > > > > > > > clean up. > > > > > > > > > > > > > > > > > > Thanks again. These are good enhancements. Appreciate > > your > > > > > > efforts > > > > > > > > here. > > > > > > > > > > > > > > > > > > Bosco > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On 8/8/19, 2:03 AM, "Rajini Sivaram" < > > rajinisiva...@gmail.com > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > Hi Don, > > > > > > > > > > > > > > > > > > Thanks for reviewing the KIP. > > > > > > > > > > > > > > > > > > 1. I had this originally as a single Action, but > > thought it > > > > > > may > > > > > > > > be > > > > > > > > > useful > > > > > > > > > to support batched authorize calls as well and keep > > it > > > > > > > > consistent with > > > > > > > > > other methods. Single requests can contain multiple > > topics. > > > > > > For > > > > > > > > > example a > > > > > > > > > produce request can contain records for several > > partitions > > > > > of > > > > > > > > different > > > > > > > > > topics. Broker could potentially authorize these > > together. > > > > > > For > > > > > > > > > SimpleAclAuthorizer, batched authorize methods don't > > > > > provide > > > > > > > any > > > > > > > > > optimisation since lookup is based on resources > > followed by > > > > > > the > > > > > > > > > matching > > > > > > > > > logic. But some authorizers may manage ACLs by user > > > > > principal > > > > > > > > rather > > > > > > > > > than > > > > > > > > > resource and may be able to optimize batched > > requests. I am > > > > > > ok > > > > > > > > with > > > > > > > > > using > > > > > > > > > single Action if this is likely to cause issues. > > > > > > > > > 2. If you have two listeners, one for inter-broker > > traffic > > > > > > and > > > > > > > > another > > > > > > > > > for > > > > > > > > > external clients, start method is invoked twice, > > once for > > > > > > each > > > > > > > > > listener. On > > > > > > > > > second thought, that may be confusing and a single > > start() > > > > > > > > invocation > > > > > > > > > that > > > > > > > > > provides all listener information and returns > > multiple > > > > > > futures > > > > > > > > would be > > > > > > > > > better. Will update the KIP. > > > > > > > > > 3. A typical example is a consumer subscribing to a > > regex > > > > > > > > pattern. We > > > > > > > > > request all topic metadata from the broker in order > > to > > > > > decide > > > > > > > > whether > > > > > > > > > the > > > > > > > > > pattern matches, expecting to receive a list of > > authorised > > > > > > > > topics. The > > > > > > > > > user > > > > > > > > > is not asking to subscribe to an unauthorized topic. > > If > > > > > there > > > > > > > > are 10000 > > > > > > > > > topics in the cluster and the user has access to 100 > > of > > > > > them, > > > > > > > at > > > > > > > > the > > > > > > > > > moment > > > > > > > > > we log 9900 DENIED log entries at INFO level in > > > > > > > > SimpleAclAuthorizer. > > > > > > > > > The > > > > > > > > > proposal is to authorize this request with > > > > > > > > AuthorizationMode.FILTER, so > > > > > > > > > that authorizers can log resources that are filtered > > out at > > > > > > > > lower level > > > > > > > > > like DEBUG since this is not an attempt to access > > > > > > unauthorized > > > > > > > > > resources. > > > > > > > > > Brokers already handle these differently since no > > > > > > authorization > > > > > > > > error > > > > > > > > > is > > > > > > > > > returned to the client in these cases. Providing > > > > > > authorization > > > > > > > > mode to > > > > > > > > > authorizers enables authorizer implementations to > > generate > > > > > > > > better audit > > > > > > > > > logs. > > > > > > > > > 4. Each request may contain multiple instances of > > the same > > > > > > > > authorizable > > > > > > > > > resource. For example a produce request may contain > > records > > > > > > for > > > > > > > > 10 > > > > > > > > > partitions of the same topic. At the moment, we > > invoke > > > > > > > authorize > > > > > > > > > method 10 > > > > > > > > > times. The proposal is to invoke it once with > > count=10. The > > > > > > > > count is > > > > > > > > > provided to authorizer just for audit logging > > purposes. > > > > > > > > > 5. Authorizer implements Closeable, so you could use > > > > > close() > > > > > > to > > > > > > > > flush > > > > > > > > > audits? > > > > > > > > > > > > > > > > > > On Thu, Aug 8, 2019 at 7:01 AM Don Bosco Durai < > > > > > > > bo...@apache.org > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > Rajini > > > > > > > > > > > > > > > > > > > > Thanks for putting this together. It is looking > > good. I > > > > > > have > > > > > > > > few > > > > > > > > > > questions... > > > > > > > > > > > > > > > > > > > > 1. List<AuthorizationResult> authorize(..., > > List<Action> > > > > > > > > actions). > > > > > > > > > Do you > > > > > > > > > > see a scenario where the broker will call > > authorize for > > > > > > > > multiple > > > > > > > > > topics at > > > > > > > > > > the same time? I can understand that during > > > > > > creating/deleting > > > > > > > > ACLS, > > > > > > > > > > multiple permissions for multiple resources might > > be > > > > > done. > > > > > > > For > > > > > > > > > authorize > > > > > > > > > > call, would this be a case? And does the Authorize > > > > > > > > implementation > > > > > > > > > will be > > > > > > > > > > able to do performance optimization because of > > this? Or > > > > > > > should > > > > > > > > we > > > > > > > > > just keep > > > > > > > > > > it simple? I don't see it as an issue from Apache > > Ranger > > > > > > > side, > > > > > > > > but > > > > > > > > > just > > > > > > > > > > checking to see whether we need to be aware of > > something. > > > > > > > > > > 2. Should I assume that the SecurityProtocol passed > > > > > during > > > > > > > > start and > > > > > > > > > the > > > > > > > > > > one return by > > KafkaRequestContext.securityProtocol() will > > > > > > be > > > > > > > > the > > > > > > > > > same? > > > > > > > > > > CompletableFuture<Void> start(String listenerName, > > > > > > > > SecurityProtocol > > > > > > > > > > securityProtocol); > > > > > > > > > > KafkaRequestContext.securityProtocol() > > > > > > > > > > 3. What is the purpose of AuthorizationMode? How > > does the > > > > > > > > broker > > > > > > > > > decide > > > > > > > > > > what mode to use when the authorize() method is > > called? > > > > > > > > > > 4. Can we clarify "count" in Action a bit more? > > How is it > > > > > > > used? > > > > > > > > > > 5. Do you feel having "stop" along with "start" be > > > > > helpful? > > > > > > > > E.g. In > > > > > > > > > Ranger > > > > > > > > > > we try to optimize the Audit writing by caching > > the logs > > > > > > for > > > > > > > a > > > > > > > > fixed > > > > > > > > > > interval. But when the Broker terminates, we do a > > forced > > > > > > > flush. > > > > > > > > > Having an > > > > > > > > > > explicit "stop" might give us a formal way to > > flush our > > > > > > > audits. > > > > > > > > > > > > > > > > > > > > Thanks > > > > > > > > > > > > > > > > > > > > Bosco > > > > > > > > > > > > > > > > > > > > On 8/7/19, 3:59 PM, "Rajini Sivaram" < > > > > > > > rajinisiva...@gmail.com > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > Hi Ron/Harsha/Satish, > > > > > > > > > > > > > > > > > > > > Thanks for reviewing the KIP! > > > > > > > > > > > > > > > > > > > > We should perhaps have a wider discussion > > outside > > > > > this > > > > > > > KIP > > > > > > > > for > > > > > > > > > > refactoring > > > > > > > > > > clients so that others who are not following > > this KIP > > > > > > > also > > > > > > > > > notice the > > > > > > > > > > discussion. Satish, would you like to start a > > > > > > discussion > > > > > > > > thread > > > > > > > > > on dev? > > > > > > > > > > > > > > > > > > > > Regards, > > > > > > > > > > > > > > > > > > > > Rajini > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Aug 7, 2019 at 6:21 PM Satish Duggana < > > > > > > > > > > satish.dugg...@gmail.com> > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > I felt the same need when we want to add a > > > > > pluggable > > > > > > > API > > > > > > > > for > > > > > > > > > core > > > > > > > > > > > server functionality. This does not need to > > be part > > > > > > of > > > > > > > > this > > > > > > > > > KIP, it > > > > > > > > > > > can be a separate KIP. I can contribute those > > > > > > > refactoring > > > > > > > > > changes if > > > > > > > > > > > others are OK with that. > > > > > > > > > > > > > > > > > > > > > > It is better to have a structure like below. > > > > > > > > > > > > > > > > > > > > > > kafka-common: > > > > > > > > > > > common classes which can be used in any of > > the > > > > > other > > > > > > > > modules > > > > > > > > > in Kafka > > > > > > > > > > > like client, Kafka-server-common and server > > etc. > > > > > > > > > > > > > > > > > > > > > > kafka-client-common: > > > > > > > > > > > common classes which can be used in the > > client > > > > > > module. > > > > > > > > This > > > > > > > > > can be > > > > > > > > > > > part of client module itself. > > > > > > > > > > > > > > > > > > > > > > kafka-server-common: > > > > > > > > > > > classes required only for kafka-server. > > > > > > > > > > > > > > > > > > > > > > Thanks. > > > > > > > > > > > Satish. > > > > > > > > > > > > > > > > > > > > > > On Wed, Aug 7, 2019 at 9:28 PM Harsha > > Chintalapani > > > > > < > > > > > > > > > ka...@harsha.io> > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > Thanks for the KIP Rajini. > > > > > > > > > > > > Quick thought, it would be good to have a > > common > > > > > > > module > > > > > > > > > outside of > > > > > > > > > > > clients > > > > > > > > > > > > that only applies to server side > > interfaces & > > > > > > > changes. > > > > > > > > It > > > > > > > > > looks > > > > > > > > > > like we > > > > > > > > > > > are > > > > > > > > > > > > increasingly in favor of using Java > > interface for > > > > > > > > pluggable > > > > > > > > > > modules on > > > > > > > > > > > the > > > > > > > > > > > > broker side. > > > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > Harsha > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Aug 06, 2019 at 2:31 PM, Rajini > > Sivaram < > > > > > > > > > > rajinisiva...@gmail.com > > > > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > > > > > > > > > > > > > I have created a KIP to replace the Scala > > > > > > > Authorizer > > > > > > > > API > > > > > > > > > with a > > > > > > > > > > new > > > > > > > > > > > Java > > > > > > > > > > > > > API: > > > > > > > > > > > > > > > > > > > > > > > > > > - > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/ > > > > > > > > > > > > > > > KIP-504+-+Add+new+Java+Authorizer+Interface > > > > > > > > > > > > > > > > > > > > > > > > > > This is replacement for KIP-50 which was > > > > > accepted > > > > > > > > but never > > > > > > > > > > merged. > > > > > > > > > > > Apart > > > > > > > > > > > > > from moving to a Java API consistent > > with other > > > > > > > > pluggable > > > > > > > > > > interfaces > > > > > > > > > > > in the > > > > > > > > > > > > > broker, KIP-504 also attempts to address > > known > > > > > > > > limitations > > > > > > > > > in the > > > > > > > > > > > > > authorizer. If you have come across other > > > > > > > > limitations that > > > > > > > > > you > > > > > > > > > > would > > > > > > > > > > > like > > > > > > > > > > > > > to see addressed in the new API, please > > raise > > > > > > these > > > > > > > > on the > > > > > > > > > > discussion > > > > > > > > > > > > > thread so that we can consider those > > too. All > > > > > > > > suggestions > > > > > > > > > and > > > > > > > > > > feedback > > > > > > > > > > > are > > > > > > > > > > > > > welcome. > > > > > > > > > > > > > > > > > > > > > > > > > > Thank you, > > > > > > > > > > > > > > > > > > > > > > > > > > Rajini > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
