Repository navigation
fix(ipa): preserve resource scope in Operations resource operation IDs - #1414
Conversation
A collection-scoped Operations resource and an instance-scoped Operations
resource under the same parent produced identical operation IDs.
generateOperationID removes path parameters before building the noun list and
then singularizes every non-final noun, so both scopes reduced to the same
nouns.
Keep the parent noun plural when the trailing 'operations' section is attached
to a resource collection, so the two resources get distinct operation IDs:
/groups/{groupId}/clusters/operations -> listGroupClustersOperations
/groups/{groupId}/clusters/{clusterName}/operations -> listGroupClusterOperations
/groups/{groupId}/clusters/operations/{operationId} -> getGroupClustersOperation
/groups/{groupId}/clusters/{clusterName}/operations/{operationId} -> getGroupClusterOperation
The scope distinction applies to every method, so pin the Create, Update and
Delete shapes too. IPA-132 Operations resources are read-only and those methods
should not exist, but the tests guard consistency if they ever do.
Document the exception in the IPA-104 and IPA-105 valid-operation-id
descriptions, which govern the Get and List methods, and regenerate the
ruleset README.
Jira ticket: CLOUDP-429467
e55e479 to
6199074
Compare
yelizhenden-mdb
left a comment
yelizhenden-mdb
left a comment
There was a problem hiding this comment.
Waiting for DevTools team confirmation
| The Operation ID must start with the verb “get” and should be followed by a noun or compound noun. | ||
| The noun(s) in the Operation ID should be the collection identifiers from the resource identifier in singular form. | ||
| If the resource is a singleton resource, the last noun may be the plural form of the collection identifier. | ||
| For a collection-scoped Operations resource, such as '/clusters/operations/{operationId}', the parent noun is in plural form, so that the Operation ID differs from the one for the instance-scoped Operations resource, such as '/clusters/{clusterName}/operations/{operationId}'. |
There was a problem hiding this comment.
Operation IDs for collection-scoped and instance-scoped LRO resources are already expected to be different — as are all operation IDs, since they map to CLI command names. Restating that in an operation-ID validity check doesn't add value, especially since the rule doesn't actually verify cross-path uniqueness. If you want to document the change, it's enough to say what the rule now expects:
For a collection-scoped Operations resource, such as /resources/operations/{operationId}, the parent noun (e.g. resources) stays in its plural form instead of being singularized
There was a problem hiding this comment.
Same applies to IPA-105
There was a problem hiding this comment.
Fixed: reworded to describe what the rule expects instead of restating uniqueness.
| ).toEqual('getGroupClusterOperation'); | ||
| }); | ||
|
|
||
| it('should generate distinct operation IDs for collection- and instance-scoped operations resources', () => { |
There was a problem hiding this comment.
Do we need this test, given that we already have the previous check?
There was a problem hiding this comment.
Fixed: removed redundant tests.
| }); | ||
|
|
||
| it('should not preserve the parent plural for other operations paths', () => { | ||
| // the parent is a single resource, so the existing singularization applies |
There was a problem hiding this comment.
What does "the parent is a single resource" mean in this context?
We would expect to see listGroupsOperations for /api/atlas/v2/groups/operations
There was a problem hiding this comment.
Fixed: added a test for /groups/operations (no ID) — confirms it hits the collection-scoped branch and returns listGroupsOperations as expected. Also reworded the adjacent comment to name {groupId} as thats what makes the other case instance-scoped.
…ant test; add collection-scoped coverage - Reword IPA-104/IPA-105 descriptions per review feedback - Regenerate README.md - Remove redundant distinct-operation-ID test (already covered by exact-value assertions) - Add test for /groups/operations (no ID) and clarify adjacent comment
3 participants
Proposed changes
Problem: LRO standardization (IPA-132) nests read-only Operations endpoints under a parent resource at both collection and instance scope. The collection- and instance-scoped pairs generated identical operation IDs:
/clusters/operationslistGroupClusterOperations/clusters/{clusterName}/operationslistGroupClusterOperations/clusters/operations/{operationId}getGroupClusterOperation/clusters/{clusterName}/operations/{operationId}getGroupClusterOperationgenerateOperationIDstrips all path parameters before building the noun list and then singularizes every non-final noun, so both scopes collapse to the same nouns. Note this affects every method on these paths, not just List — the{operationId}parameter does not make Get naturally distinct, because it is discarded along with the other path parameters.Fix: add an
isCollectionScopedOperationsPathcheck and keep the parent noun in its plural form when the trailingoperationssection is attached to a resource collection rather than to a single resource. The trailing noun keeps its normal meaning (plural for List, singular for Get), so the existing convention is preserved and all four IDs become distinct:/clusters/operationslistGroupClustersOperations/clusters/{clusterName}/operationslistGroupClusterOperations/clusters/operations/{operationId}getGroupClustersOperation/clusters/{clusterName}/operations/{operationId}getGroupClusterOperationThis reuses the existing
singularizehelper and itsignoreSingularizationListopt-out rather than introducing newCollection/Instancesuffix conventions.Documentation: the exception is documented in the
xgen-IPA-104-valid-operation-idandxgen-IPA-105-valid-operation-idrule descriptions (the Get and List methods), and the rulesetREADME.mdis regenerated to match.Verification: unchanged behavior for non-Operations paths, a singular parent (
/groups/{groupId}/operations), a non-trailingoperationssection, an unscoped/api/atlas/v2/operations, and multi-word custom methods — all covered by tests. Full suite passes (705 tests), along withlint-jsandformat-check.Because the scope distinction is a property of the path rather than of the method, the Create, Update and Delete shapes are pinned by tests as well:
create/clusters/operationscreateGroupClustersOperationcreate/clusters/{clusterName}/operationscreateGroupClusterOperationupdate/clusters/operations/{operationId}updateGroupClustersOperationupdate/clusters/{clusterName}/operations/{operationId}updateGroupClusterOperationdelete/clusters/operations/{operationId}deleteGroupClustersOperationdelete/clusters/{clusterName}/operations/{operationId}deleteGroupClusterOperationThese endpoints should not exist — IPA-132 Operations resources are read-only — so the tests are not asserting supported API surface. They are there purely to guarantee the collection-vs-instance distinction stays consistent across every method rather than being special-cased to Get and List, and to catch a silent regression if a mutating method is ever added to an Operations path.
Follows up on #1406, which relaxed
xgen-IPA-102-path-alternate-resource-name-path-paramto permit these paths.Jira ticket: CLOUDP-429467
Checklist
Changes to Spectral
Further comments
One thing worth flagging for review: there is no operation-ID uniqueness check anywhere in the repo today — each rule only validates that an operation ID matches the one generated from its own path. The collision described above would not have been flagged by validation, only by downstream SDK codegen, where each operation ID becomes a method name.