Repository navigation
feat(ipa): require long-running operations to expose an Operations endpoint - #1461
Conversation
| const basePath = isCustomMethodIdentifier(resourcePath) ? stripCustomMethodName(resourcePath) : resourcePath; | ||
|
|
||
| const candidatePaths = [basePath]; | ||
| if (isSingleResourceIdentifier(basePath)) { |
There was a problem hiding this comment.
if the path is collection identifier -> check resource/operations and resource/operations/{operationId} exists
if the path is single resource identifier -> check resource/{resourceId}/operations and resource/{resourceId}/operations/{operationId} exists
How does it sound?
There was a problem hiding this comment.
Sounds right — done in 938a514.
I had allowed a mutation of a single resource to satisfy the rule with the Operations endpoints at the parent collection as well, on the assumption that both levels were acceptable. That fallback is gone; the check is now exactly as you describe: the Operations endpoints must sit under the resource path of the mutation itself, collection or instance
Also rebased: #1448 has merged, so this is now a single commit on main and the stacked-PR note is dropped. Test updated to cover the parent-collection shape as a violation instead of a pass; full IPA suite green (861 tests).
938a514 to
72aa08e
Compare
| step, so the rule behaves identically on merged and per-service specs | ||
| - Legacy operations that predate IPA-132 are excluded from validation, whether marked | ||
| `legacy: true` during the merge step or matched by the shared legacy operationId list | ||
| - Both the Operations collection (`.../operations`) and the single Operation endpoint |
There was a problem hiding this comment.
Only Get and List methods must be defined. Tests follow this, however I couldn't see it in the rule implementation and we can update the documentation accordingly
There was a problem hiding this comment.
Good catch — you're right, that was only enforced by accident in the tests. Fixed in 7d2bb80.
The implementation checked that the two path keys existed and never looked at the methods they defined. Every test happened to write get: {} on those paths, so the suite read as if the requirement were enforced, but a spec defining /resource/operations with only post would have passed the rule.
The rule now requires the get method on both endpoints — List on the Operations collection, Get on the single Operation — and the message ends with "each with a get method". Two new invalid cases cover it: a collection defined with only post, and a single Operation defined with only delete. Both passed before, both flag now.
On the "only" half: I've left "no method other than get" to xgen-IPA-132-operation-must-be-a-read-only-resource, which already rejects it on Operations endpoints. So this rule requires get to be present and that one forbids everything else — they compose without overlapping. Happy to pull the exclusivity check in here too if you'd rather have it in one place.
Documentation updated in IPA-132.yaml with the new bullet, README regenerated from it. Suite green at 863 tests.
…dpoint Add xgen-IPA-132-lros-must-expose-an-operations-endpoint: a mutating method tagged as a compliant long-running operation must define both the Operations collection and the single Operation endpoint, nested under its resource path. A mutation of a collection exposes them under the collection path, a mutation of a single resource under the resource instance path; custom method suffixes are stripped when deriving the resource path.
72aa08e to
7d2bb80
Compare
andreaangiolillo
left a comment
andreaangiolillo
left a comment
There was a problem hiding this comment.
LGTM. Thanks for all the great contributions during your internship! 💯 🫡
Proposed changes
This PR adds the last IPA-132 rule: a long-running operation must expose the Operations endpoint clients poll for status.
Jira ticket: CLOUDP-435359
Rule added
At
warnseverity, given$.paths[*][post,put,patch,delete]:xgen-IPA-132-lros-must-expose-an-operations-endpoint.../operations) and the single Operation endpoint (.../operations/{operationId}) are defined under the resource path of the long-running operationResource path resolution: custom method suffixes are stripped before deriving the resource path. A mutation of a collection must expose the endpoints under the collection path (
/resource/operations), and a mutation of a single resource under the resource instance path (/resource/{resourceId}/operations).How long-running operations are selected
Through the shared
isNonLegacyLongRunningOperationhelper introduced in #1448 — selection is on the declared 202 response, with legacy operations excluded both by thelegacy: truemarking and by the shared legacy operationId list. No new selection logic.Testing
Valid cases for the collection-level, instance-level and custom-method shapes; invalid cases for a missing Operations endpoint, for a collection defined without the single Operation endpoint, and for an instance-level operation whose Operations endpoints sit only at the parent collection; violation-with-exception (suppressed) and compliant-with-exception (flagged as unnecessary); selection cases covering untagged operations declaring a 202 (valid and violating), synchronous operations, operations tagged without a 202, and legacy exclusion via both the
legacy: truemarking and the shared operationId list.Full suite: 131 suites / 861 tests pass. Prettier clean.
Checklist