Subject: RE: [Policy] FW: I-D ACTION:draft-reyes-policy-core-ext-schema-04 .txt

From: John Strassner <[email-masked]>
Date: Fri, 13 Feb 2004 19:31:48 -0700

First, I support Kurt's comments on LDAP, and will reply to those in a
separate email.
 
Second, I list below a set of additional comments on this draft.
 
Third, the lack of an overall diagram makes it very difficult to evaluate
the correctness of this model. This draft is not complete enough to
construct such a model.
 
Fourth, a cursory scan revealed that there is no pcelsPolicyGroup class.
This is strange, since PolicyGroup is listed as a subclass of PolicySet in
RFC 3460. Why is this?
 
Fifth, why is there a pcelsRule and a pcimRule class?
 
Sixth, why is there no pcelsRuleValidityAssociation subclass? At this point,
I started to go through the document in detail with my developers to try and
implement it. We couldn't. We give you inconsistencies that we noticed
(grammatical and otherwise) through page 31).
 
Finally, I was surprised to see a lack of an Acknowledgments section,
especially given the amount of feedback that several people on this list
gave the authors. That's in poor form.
 
Comments are as follows:
 
  - s/RFC zzzz/RFC 3703
  - page 3. You write: "...the combined class hierarchy for the LDAP 
    object classes defined in [PCLS] and in this document". You should
    include concepts from 3460 that you mapped into new classes, and
    add that you defined new classes not in 3460 or 3703.
  - page 4-7, class diagram - this diagram has no caption. Please add
    one. In addition, I find the diagram inpenetrable, in that the
    reader has no idea where these classes came from. I think you need
    a simpler introduction saying 3060 provided this, 3460 did this, and
    thus we came up with this. Take this key and show, for any class 
    that isn't new in this document, where it came from.
  - page 4 - why is your class named pcelsFilerEntry, when 3460 names
    its class FilterEntryBase?
  - page 4 - why is your class named pcelsIPHeaders, when 3460 names
    its class IPHeadersFilter? The Filter part is important! 
  - page 4 - why is your class named pcels8021Headers, when 3460 names
    its class 8021Filter? The Filter part is important!
  - page 4 - why is your class named pcelsCompoundFilterAuxClass, when 
    a more consistent name would be pcelsCompoundFilterConditionAuxClass?
    The Condition part is important!
  - general reflections on the class diagram: part of the problem is that
    you are building a schema from three different sources: (1) RFC 3703,
    (2) RFC 3460, and (3) your own additions. I see no discussion on how
    these relate to each other, which would have been helpful.
  - page 7 - you didn't state whether this is for all associations. This
    is exacerbated by you saying: "...might need to implement the 
    association..." - which implies a single association. In addition,
    this is a terse description - the naive reader won't understand why
    aux classes are being used - you need a reference or a couple of
    sentences explaining this.
  - page 7 - you state: "The LDAP object classes defined in this document
    are a direct mapping from the corresponding classes and, in some 
    cases, the associations defined in [PCIM_EXT] ". Not strictly true, 
    as you are also seeking to update RFC 3703 (e.g., where is 
    pcimSubtreesPtrAuxClass defined in RFC 3460?).
  - pages 8-11: your table has no caption
  - pages 8-11: Where are classes like pcimSubtreesPtrAuxClass? They
    aren't listed in this table, and better be, if you are "updating"
    RFC 3703.
  - once again, I see lots of irksome naming issues. The LDAP schema
    shouldn't change the name of a class defined in another RFC. Why
    have you done this?
  - page 8, 4th row. How can you give two different mappings to a single
    object class? And how can a RULE (i.e., pcelsRule) map to a GROUP?
  - page 10, 1st row. How can a single info model association map to
    two different associations? And do you mean "and" in this row? This
    would mean that I would have to instantiate both pcelsPolicySet
    and pcelsPolicySetAssociation, which is clearly wrong. This comment
    also applies for the other rows on this page where you have "and".
  - page 10 - it is of no help to say "see PolicySetInSystem" in this
    table for the 3rd and 4th rows - that only confuses the reader.
    Please spell out what you mean here.
  - Page 11 - the reader will wonder why ReusablePolicy and 
    PolicyRoleCollectionInSystem are only implementable via DIT 
    containment, when every other association has an association defined
    (independent of whether DIT containment could be used).
  - Section 4.2, line 3, you write: "The concept of an ordered set of
    policies...". LDAP doesn't have ordered sets. How are you going to
    implement this?
  - Page 13, Section 4.3, second paragraph - s/deprecates/deprecate
  - Page 13, Note - actually, PCLS does NOT have anything to do with
    PCIMe, so this note needs to be reworded
  - Page 14, Section 4.5
    - s/an other/another (and other places)
    - s/"rule /group"/"rule/group" (5 places) (and other places)
  - Page 16, section 4.6, 
    - s/CompoundPolicyActionclasses/CompoundPolicyAction classes 
    - conditions /actions/"conditions/actions" (and other places)
  - Page 22. How are you going to enforce an "ordered set of 
    rules /groups"? That is, how can you guarantee that the DSA stores
    your rules/groups [sic] in the order that you want, and where is
    that order specified? What if a DSA doesn't have ordering controls?
  - Same section as above - you say that the "association entries enable 
    relative ordering of the aggregated pcelsPolicySet instances within 
    the scope of the aggregating pcelsPolicySet" - how is this 
    accomplished with a plain, vanilla LDAP server with no controls?
  - Page 23 - the DESC for pcelsPolicySetList should say that it
    contains an UNORDERED list of DN references.
  - Page 23, note above Section 5.2, is slightly incorrect. Only those
    implementations that WANT TO BE COMPATIBLE WITH PCELS should use
    this aggregation mechanism instead of those defined by PCLS. Not
    every implementation mechanism is going to want to change.
  - Page 23, Section 5.2, says "The pcelsPolicySetAssociation class is 
    used to aggregate instances of pcelsPolicySet into other entries."
    This is incorrect, as pcelsPolicySet is abstract and thus cannot be
    instantiated.
  - Same section, you write: "...realizes a (subclass of)
    PolicySetComponent aggregation [sic]. When subordinated to (subclass 
    of) dlm1System...realizes a PolicySetInSystem association [sic]".
    How can the same element realize an aggregation in one usage and an
    association in another usage? This is semantically inconsistent.
  - Next paragraph says: "A non-reusable instance of (subclass of)
    pcelsPolicySet is attached as auxiliary class directly to the 
    pcelsPolicySetAssociation entry." Subclasses of pcelsPolicySet that
    are not abstract are pcelsRuleAuxClass and pcelsRuleInstance. The
    above sentence only makes sense for pcelsRuleAuxClass.
  - Next paragraph doesn't make sense. First, you clearly mean a non-
    abstract subclass of pcelsPolicySet. Second, you are recommending
    that an ERROR be ignored? Why don't you stop operation?
  - Page 24, DESC of pcelsPriority is insufficient, as "0" has special
    semantics that you haven't mentioned. This should, of course, also
    be present in accompanying prose, as Kurt points out.
  - Page 24, DESC of pcelsPolicySetDN should state that this is an
    UNORDERED list of DNs.
  - Page 24, Section 5.3, s/The Three Classes pcelsRule/The pcelsRule
    Class and Its Subclasses
<note: at this point I'm not going to correct any remaining grammar
 errors, such as the next line ("The pcelsRule is...") because there
 are too many of them.>
  - Page 24, Section 5.3, you say: "The pcelsRule is the base class
    representing policy rules." Does this mean that an implementation
    can NOT use the subclasses of pcimRule anymore?
  - Page 24, next paragraph, you say: "This class shares the 
    Condition/Action aggregation methods with the
    pcelsCompoundConditionAuxClass and pcelsCompoundActionAuxClass
    object classes.". Why does it also not share the
    pcelsSimpleConditionAuxClass and pcelsSimpleActionAuxClass
    object classes as well?
  - Page 25, top paragraph, again says that the implementer should ignore
    an error condition. This isn't a good idea.
  - Page 25, next paragraph has the same problem.
  - Page 26, the pcelsConditionListType attribute has a constraint. No
    text is provided that instructs the implementer what to do, aside
    from Note 5 on page 21, which says: "Text has been added to instruct
    servers and applications what to do if a value outside of this range
    is encountered" - which is exactly the problem - no text is here.
Note that this is a systemic problem with any constrained attribute
defined in this draft. Thus, I will only mention this once.
  - Page 27, WHY isn't a PolicyGroup class implemented? You give no
    reason for not doing this. Note also that Note 2 talks about ORDERED
    policy rules - I don't see how you can construct those.
  - Page 27, section 5.4, again you say "pcelsRule" instead of "non-
    abstract subclasses of pcelsRule".
  - Page 28, top paragraph, another error that you are recommending 
    should be ignored
  - Page 28, DESC for pcelsConditionAssociation is wrong; you say that
    it can be used for a pcelsRule instead of a non-abstract subclass of
    pcelsRule
  - Page 28, section 5.5, again you say "pcelsRule" instead of "non-
    abstract subclasses of pcelsRule".
  - Page 28, last paragraph, another error that you are recommending
    should be ignored.
  - Page 29, DESC for pcelsActionAssociation is wrong; you say that 
    it can be used for a pcelsRule instead of a non-abstract subclass of
    pcelsRule
  - Page 29, last paragraph above Section 5.6, another error that you are
    recommending should be ignored.
  - Page 29, Section 5.6, last two paragraphs are errors that you are
    recommending should be ignored.
<At this point, I'm going to stop listing these, as it is a systemic problem
that should be fixed in the next release>
  - Page 29, last paragraph above Section 5.6, another error that you are
    recommending should be ignored.
  - Page 30, the DESC for pcelsVariableDN is wrong. You say that it is a
    "DN reference to a pcelsVariable entry", when it should be a DN
    reference to a subclass of either pcelsExplicitVariableAuxClass or
    pcelsImplicitVariableAuxClass or pcelsVendorVariableAuxClass
  - Page 30, the DESC for pcelsValueDN is wrong - it should be a subclass
    of pcelsValueDN.

regards,
John 

John C. Strassner 
Chief Strategy Officer 
Intelliden Inc. 
90 South Cascade Avenue 
Colorado Springs, CO  80906  USA 
phone:  +1.719.785.0648 
  fax:     +1.719.785.0644 
email:    [email-masked]
<mailto:[email-masked]>