[
https://issues.apache.org/jira/browse/SLING-9871?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17306210#comment-17306210
]
Ashish Chopra commented on SLING-9871:
--------------------------------------
hi [~bdelacretaz], Apologies but I'm not sure what sort of updates can I make
to the issue.
The problem statement is specifically for ACEs, for which the order of their
application matters greatly. I must admit I'm not aware whether similar
ordering-concerns have been raised for other directives as well.
AIU,
* bundle-inclusion directives have start level and bundles themselves have
package-imports to determine starting order at runtime
* content-packages have a way to specify dependencies on other
content-packages, which determines installation order
* path creation operations [create parents if
required|https://github.com/apache/sling-org-apache-sling-jcr-repoinit/blob/760547d538b6ba3357a9baf25efe50c09b734bc7/src/test/java/org/apache/sling/jcr/repoinit/CreatePathsTest.java#L48-L52]
* node-type registration and service-user creation is independent of the order
of application
* group membership updates _should_ not be defined such as to clash one another
as a best-practice, and thus independent of the order of application
* setting properties at paths _does_ rely on the order of application
* setting ACEs at paths _does_ rely on the order of application
* (not sure if there are other operations I have missed enumerating here)
Based on the items I've enumerated above, I think {{set properties}} and {{set
ACL}} are the only ones that can benefit from a
aggregation-order-specification-in-feature-files-through-dependencies.
To me, enhancing repoinit to specify (and enforce) "the order in which
fragments are arranged by the feature-aggregator" _should_ help the situation
this issue raises, but right now seems an overkill to address it - however, it
is quite likely that given my lack of complete understanding of how
feature-aggregation and repoinit interact (or maybe additiona repoinit
usecases) I'm not able to fully grasp the value such a specification and
enforcement would add.
bq. my understanding of the problematic use case is [as follows
[here|https://issues.apache.org/jira/browse/SLING-9871?focusedCommentId=17306055&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17306055]]
bq. [summarily] you need some of the entries of S to come before or after E
Ok, that's not the case I was alluding to - as the issue description (and a
follow-up
[comment|https://issues.apache.org/jira/browse/SLING-9871?focusedCommentId=17304635&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17304635])
notes, the ACEs amongst which the ordering is a concern are all defined within
the feature aggregate and are processed by repoinit (whether they have been
already applied in repository or not is not relevant).
The case you describe is interesting - and I'd think that the scenarios where
this will become a problem would be only when a downstream consumer of the
platform (custom-code, or end-user) manipulates product-defined ACEs. I'd think
the inability to specify the order of ACEs independent of
"order-of-feature-aggregation" is a bigger problem.
It'd be good if we can come up with a pattern to solve ordering with
* ACEs processed by repoinit + existing ACEs in the repository, and
* ACEs processed by reponit
can be addressed - but if it gets overly complicated, solving for the latter is
more important than one for the former IMHO.
bq. we now support remove *, removing all existing entries and setting a
complete new set in the right order (as defined by the order of repoinit
statements) might also be an option, or at least a workaround.
Thanks, I didn't know about this! Thinking through a bit, though, I see it
suffers from the same ownership issues I described earlier. Since the order of
ACE application is coupled with order of feature-aggregation, the {{remove *}}
directive may be applied in a feature-file that is not processed at the desired
time (i.e., the very last feature-file to be processed), thus deviating from
the set the developer expects v/s the one that appears in the repository.
bq. there one piece missing in the discussion here: the default implementation
in Oak will perform some automatic cleanup and
AccessControlList.addAccessControlEntry (and the jackrabbit variants of it) are
not guaranteed to result in a new ACE being added at the end of the list (see
oak docu and exercises for examples)...the re-ordering might only work under
special circumstances..... so I am not sure this is doable except for very
limited cases.
Thanks for this extremely valuable information! I was looking at repoinit to
mirror what [Filevault
does|https://github.com/apache/jackrabbit-filevault/blob/f785fcb24d4cbd01c734e9273310a925c29ae15b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/JackrabbitACLImporter.java#L229-L321]
without the complex [merging
rules|http://jackrabbit.apache.org/filevault/apidocs/org/apache/jackrabbit/vault/fs/io/AccessControlHandling].
As you mentioned, repoinit is using
[JackrabbitAccessControlList|https://github.com/apache/sling-org-apache-sling-jcr-repoinit/blob/611be914745027ddd6d99e0d963867a4a48bf20a/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java#L127-L162]
- I wonder if a combination of {{AccessControlUtils.getAccessControlList}} and
{{JackrabbitAccessControlList.orderBefore}} will allow us to perform the
reordering, or do you think there's no way to implement a definitive-reorder of
the ACEs repoinit adds by any means?
> Specifying order of ACEs through repoinit directives
> ----------------------------------------------------
>
> Key: SLING-9871
> URL: https://issues.apache.org/jira/browse/SLING-9871
> Project: Sling
> Issue Type: Improvement
> Components: Repoinit
> Reporter: Ashish Chopra
> Priority: Major
>
> As of writing this, repoinit processor (among other things not relevant to
> this JIRA) collects {{create path}} statements and {{set ACL}} statements
> declared in all the feature-models applicable to feature-aggregate under
> consideration.
> Upon repository initialization, it applies all the {{create path}}
> statements, followed by all the {{set ACL}} statements. However, the order in
> which {{set ACL}} statements declared across feature models are applied isn't
> defined (currently, it seems to be based on feature-model-name,
> alphabetically ascending).
> This causes issues at times because we want the order of the ACEs to be
> maintained (e.g., "deny"s for everyone at a given path must be the first ACE,
> followed by "allow"s for specific, non-system-user principals)
> Repoinit should be able to support this requirement.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
