DCL API#123
Conversation
|
Preview deployed at https://d2doqw88td69eq.cloudfront.net/pr-123 |
There was a problem hiding this comment.
Code Review
This pull request introduces a new, well-structured API reference page for the DCL and links to it from the main DCL documentation. The new page provides a helpful overview of the DCL REST API. My review includes suggestions to enhance the consistency and completeness of this new documentation, such as shortening lengthy section titles for better readability, adding missing JSON response examples to provide a more comprehensive guide for developers, and correcting a minor formatting inconsistency.
|
Preview deployed at https://d2doqw88td69eq.cloudfront.net/pr-123 |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
Preview deployed at https://d2doqw88td69eq.cloudfront.net/pr-123 |
|
Preview deployed at https://d2doqw88td69eq.cloudfront.net/pr-123 |
|
Preview deployed at https://d2doqw88td69eq.cloudfront.net/pr-123 |
1 similar comment
|
Preview deployed at https://d2doqw88td69eq.cloudfront.net/pr-123 |
|
Preview deployed at https://d2doqw88td69eq.cloudfront.net/pr-123 |
|
Preview deployed at https://d2doqw88td69eq.cloudfront.net/pr-123 |
|
|
||
| ### REST API | ||
| * **Query All Root Certificates:** `/dcl/pki/root-certificates` | ||
| * **Returns:** The list of all approved PAA certificates. |
There was a problem hiding this comment.
Q for DCL folks - are the signing keys for the CDs also in here?
There was a problem hiding this comment.
Private keys are never put on chain.
There was a problem hiding this comment.
Bad phrasing - what I was attempting to ask was whether the certificates for the CD signing keys in this portion of the DCL.
| **Description:** Provides the list of Operational Root CA Certificates (RCAC) and Operational Intermediate CA Certificates (ICAC). | ||
|
|
||
| ### REST API | ||
| * **Status:** Under Development |
There was a problem hiding this comment.
Q for the DCL folks - I think this might correspond to the noc-root-certificates / noc-ica-certificates, but there are some missing fields.
It's also not clear how the -vid- tables factor in here.
There was a problem hiding this comment.
vid-specific queries are just helper queries to query RCACs/ICACs by VID (instead of common querying by subject/SKID as in Device Attestation certificates).
Please see #123 (comment) for details.
|
Preview deployed at https://d2doqw88td69eq.cloudfront.net/pr-123 |
|
|
||
| ## Operational Trust Anchors Schema | ||
| **Description:** Provides the list of Operational Root CA Certificates (RCAC) and Operational Intermediate CA Certificates (ICAC). | ||
|
|
There was a problem hiding this comment.
Rest endpoint for this is /dcl/pki/noc-ica-certificates
Returns the list of all RCAC and ICAC certificates and associated Vendor ID
Pagination is required
To query by Vendor id the following endpoint /dcl/pki/noc-vid-ica-certificates/{vid} can be used
Returns the list of RCAC and ICAC certificates and associated Vendor ID
Pagination is not applicable
There was a problem hiding this comment.
The JSON ref for that doesn't match the spec description. It lists
"certificateType": "DeviceAttestationPKI",
and is missing the IsVidVerificationSigner. The name matches, but this doesn't seem like an implementation of that spec schema.
| --- | ||
|
|
||
| ## Additional Schemas & Indices | ||
|
|
There was a problem hiding this comment.
We may also mention /dcl/pki/all-certificates/.... endpoints which return both DA and NOC based certificates.
| --- | ||
|
|
||
| ## Operational Trust Anchors Schema | ||
| **Description:** Provides the list of Operational Root CA Certificates (RCAC) and Operational Intermediate CA Certificates (ICAC). |
There was a problem hiding this comment.
Looks like exact endpoints are not mentioned here:
- /dcl/pki/all-noc-certificates
- /dcl/pki/all-noc-certificates/{subject}
- /dcl/pki/all-noc-certificates/{subject}/{subject_key_id}
- /dcl/pki/noc-vid-root-certificates/{vid}
- /dcl/pki/noc-vid-certificates/{vid}/{subject_key_id}
- /dcl/pki/noc-ica-certificates
- /dcl/pki/noc-vid-ica-certificates/{vid}
There was a problem hiding this comment.
See comment below - it's not clear to me how these relate to the spec schema and they're currently unpopulated. I'm going to leave this as a follow up for the DCL team because I genuinely don't understand the division of mapping here. This should be written by someone who can properly explain it.
There was a problem hiding this comment.
All the endpoints I mentioned are for the schema https://github.com/CHIP-Specifications/connectedhomeip-spec/blob/master/src/service_device_management/DistributedComplianceLedger.adoc#5-operational-trust-anchors-schema.
NOC== Operational Trust Anchors (soall-nocmeans bothRCACandICAC)RCAC==noc-rootICAC==noc-ica- vid-specific queries are just helper queries to query RCACs/ICACs by VID (instead of common querying by subject/SKID as in Device Attestation certificates).
As part of the current task to review changes between DCL implementation and the spec, we are going to clarify the details and either fix the difference between the spec and the code, or provided better mapping/clarifications.
There was a problem hiding this comment.
OK, until that point, I would hesitate to document this endpoint as implementing the spec schema. If the DCL team wants to come back later update this section, that should be done after the cleanup.
There was a problem hiding this comment.
I would prefer to leave this to the DCL team to update once those fixes are made.
| * **Query by Subject & SubjectKeyID:** `/dcl/pki/certificates/{subject}/{subjectKeyId}` | ||
| * **Returns:** A specific certificate. | ||
| * **Pagination:** Not applicable. | ||
| * NOTE - subject needs to be percent formatted, which is a transformation of the data returned from the base root-certificates request. |
There was a problem hiding this comment.
We may also mention
- /dcl/pki/certificates?subjectKeyId={subjectKeyId}
- /dcl/pki/certificates/{subject}
|
|
||
| * **Query by VID, PID & Software Version:** `/dcl/compliance/certified-models/{vid}/{pid}/{softwareVersion}/{certificationType}` | ||
| * **Returns:** A boolean-like record indicating if the specific version is certified. | ||
|
|
There was a problem hiding this comment.
We may also mention revoked models:
- /dcl/compliance/revoked-models
- /dcl/compliance/revoked-models/{vid}/{pid}/{software_version}/{certification_type}
There was a problem hiding this comment.
Right now there are no revoked models, and I'm not sure exactly what the corresponds to in the spec. I'd rather leave that as a follow up until a person from the DCL team can come and explain how that actually relates to the spec and what's intended to be in there.
There was a problem hiding this comment.
It's perhaps one of the DCL feature which is not mentioned explicitly in the spec. Will be updated as part of the current Spec Compliance task.
There was a problem hiding this comment.
OK, let's leave this as is, and it can be added to this documentation once the spec work is done.
|
|
||
| ### Web Interface | ||
| * **Location:** Accessed via the **Models** view details page. | ||
|
|
There was a problem hiding this comment.
There is also a way to return a list of Compliance records by cDCertificateId:
/dcl/compliance/device-software-compliance/{cDCertificateId}
|
|
||
| * **Query by VID:** `/dcl/model/models/{vid}` | ||
| * **Returns:** All models associated with a specific Vendor ID. | ||
| * **Pagination:** **Required**. |
There was a problem hiding this comment.
| * **Pagination:** **Required**. | |
| * **Pagination:** **Not applicable**. |
There was a problem hiding this comment.
Oh, is there actually not pagination there? There are definitely vendors with more than 100 models. How do those work?
There was a problem hiding this comment.
Currently that endpoint does not return models but vendor products(which contains info about name, partNumber and pid).
I hope in the upcoming release that should be changed to return all models associated with Vendor ID
There was a problem hiding this comment.
I think we're just using different words since the spec and the implementation wording is different. This endpoint returns information about vendor products. There are some vendors that have > 100 products. So are they all returned or are the results truncated?
There was a problem hiding this comment.
Yes, it returns all vendor products at once
There was a problem hiding this comment.
I don't understand the differentiation between models and products here. Can you explain the difference? The endpoint name is models, but that seems to be a reference to the various PIDs. These terms seen interchangeable.
| * NOTE - `vid` (Vendor ID) is a decimal value | ||
|
|
||
| * **Query by VID & PID:** `/dcl/model/models/{vid}/{pid}` | ||
| * **Returns:** Details for a specific product. |
There was a problem hiding this comment.
Maybe
| * **Returns:** Details for a specific product. | |
| * **Returns:** Details for a specific device model. |
There was a problem hiding this comment.
what is a device mode here? That seems like a DCL term.
There was a problem hiding this comment.
I believe Device Model is a term from the spec, and the name of the Schema: https://github.com/CHIP-Specifications/connectedhomeip-spec/blob/master/src/service_device_management/DistributedComplianceLedger.adoc#6-devicemodel-schema
There was a problem hiding this comment.
yes, and those relate "A unique combination of the VendorID and ProductID is used to identify a DeviceModel." So product and device model seem interchangeable in this context. Why is product incorrect here?
Co-authored-by: Abdulbois <30406506+Abdulbois@users.noreply.github.com>
|
Preview deployed at https://d2doqw88td69eq.cloudfront.net/pr-123 |
|
Preview deployed at https://d2doqw88td69eq.cloudfront.net/pr-123 |
|
Preview deployed at https://d2doqw88td69eq.cloudfront.net/pr-123 |
Co-authored-by: Abdulbois <30406506+Abdulbois@users.noreply.github.com>
|
Preview deployed at https://d2doqw88td69eq.cloudfront.net/pr-123 |
No description provided.