[apps-discuss] Appsdir review for draft-ietf-cdni-control-triggers-06

[Carsten Bormann <[e-mail]>]

I have been selected as an Applications Area Directorate reviewer for
this draft (for background on appsdir, please see
http://trac.tools.ietf.org/area/app/trac/wiki/ApplicationsAreaDirectorate
), with a specific view on the JSON usage in this specification.

Please resolve these comments along with any other comments you may
receive. Please wait for direction from your document shepherd or AD
before posting a new version of the draft.  (Feel free to forward
these comments to the WG list if that helps in resolving the comments.)

Document:  draft-ietf-cdni-control-triggers-06
Title: CDNI Control Interface / Triggers
Reviewer: Carsten Bormann
Review Date: 2015-04-06

* Summary: This draft is ready for publication as a standards track RFC,
after a few misphrasings and minor details in the usage of HTTP have
been corrected.

* Major issues: None.

* Minor issues:

The protocol is REST-based in that it creates dCDN activities using a
POST-based interface, which then can be examined using GET on the
location returned (making good use of HTTP, e.g., for caching) and be
DELETEd for canceling them.  It is not making use of the fact that the
subject of these activities are REST resources themselves.  This
paragraph just takes note of this fact, without necessarily suggesting
a need (or even usefulness) for a change.

Is "authenticate" the right word in the 2nd para of 4.1?
(How do you authenticate a CI/T command?  There is no authentication
information provided.)

For the benefit of the implementer, the last paragraph of section 4.1
could say what is "an appropriate HTTP status code" here -- 405 Method
not allowed?

4.2.1 could give a little additional guidance on how to use the ETag
for the collection (possibly pointing to the examples).

4.3 uses 403 forbidden to indicate that a feature is not implemented?

4.6 the phrasing "into the cdn-path key" used here is misleading: The
intent is to have one array element appended to the end of the array
under the entry named "cdn-path".

4.7 uses 401 for no permission, while section 8.1 correctly proposes
403 Forbidden (or 404 Not Found) for this case.

5.1.2 uses the phrasing "list of" Error Descriptions, apparently with
the intention that this be an array of Error Descriptions.  Prefer
making this explicit.  Are empty lists allowed?  (The whole entry is
non-mandatory.)

(More generally, the spec could be explicit where empty arrays are
allowed and where not.)

5.1.3: Is staleresourcetime allowed to be negative?  zero?
(Note also that Absolute Time is a general JSON number, which includes
floating point, while this is an integer?)

5.2.6 fails to say that Error Descriptions are JSON objects.

Why is the third request in 6.2.4 not using an ETag, contrary to
its own recommendation?

* Gratuitous formalization of the JSON data:

(I have validated the examples against this formalization, using the
CDDL tool.)


CIT-object = CIT-command / Trigger-Status-Resource / Trigger-Collection

CIT-command ; use media type application/cdni.ci.TriggerCommand+json
 = {
  ? trigger: Triggerspec
  ? cancel: [* URI]
  cdn-path: [* Cdn-PID]
}

Trigger-Status-Resource ; application/cdni.ci.TriggerStatus+json.
 = {
  trigger: Triggerspec
  ctime: Absolute-Time
  mtime: Absolute-Time
  ? etime: Absolute-Time
  status: Trigger-Status
  ? errors: [* Error-Description]
}

Trigger-Collection ; application/cdni.ci.TriggerCollection+json
 = {
  triggers: [* URI]
  ? staleresourcetime: int ; time in seconds
  ? coll-all: URI
  ? coll-pending: URI
  ? coll-active: URI
  ? coll-complete: URI
  ? coll-failed: URI
  ? cdn-id: Cdn-PID
}

Triggerspec = { ; 5.2.1
  type: Trigger-Type
  ? metadata.urls: [* URI]
  ? content.urls: [* URI]
  ? content.ccid: [* Ccid]
  ? metadata.patterns: [* Pattern-Match]
  ? content.patterns: [* Pattern-Match]
}

Trigger-Type = "preposition" / "invalidate" / "purge" ; 5.2.2

Trigger-Status = "pending" / "active" / "complete" / "processed"
   / "failed" / "cancelling" / "cancelled" ; 5.2.3

Pattern-Match = { ; 5.2.4
  pattern: tstr
  ? case-sensitive: bool
  ? match-query-string: bool
}

Absolute-Time = number ; seconds since UNIX epoch, 5.2.5

Error-Description = { ; 5.2.6
  error: Error-Code
  ? metadata.urls: [* URI]
  ? content.urls: [* URI]
  ? metadata.patterns: [* Pattern-Match]
  ? content.patterns: [* Pattern-Match]
  ? description: tstr
}

Error-Code = "emeta" / "econtent" / "eperm" / "ereject"
   / "ecdn" / "ecancelled"  ; 5.2.7

Ccid = tstr ; see I-D.ietf-cdni-metadata

Cdn-PID = tstr .regexp "AS[0-9]+:[0-9]+"

URI = tstr