search

Azure / azure-openapi-validator

Azure Open API Validator
MIT License
47 stars 46 forks source link

The BodyTopLevelProperties rule is mistakenly flagging paged responses #722

Open markcowl opened 2 months ago

markcowl commented 2 months ago

Describe the bug The BodyTopLevelProperties check is incorrectly flagging the response in List operations, which rturn a page of data as if they were resource GET operations. There are several examples in this PR: https://github.com/Azure/azure-rest-api-specs-pr/pull/18696 To Reproduce Steps to reproduce the behavior: Create list operations like: https://github.com/Azure/azure-rest-api-specs-pr/blob/a6bade603572212620a6cbb906f400e19d899b1b/specification/edge/resource-manager/Microsoft.Edge/configurationmanager/preview/2024-06-01-preview/configurationmanager.json#L62-L99

Expected behavior This check should not be applied to list operations at all - in ARM specs, having an odd number of PATH segments and a last segment that is not a path variable should prevent the rule from being applied

Screenshots See the link above.

Desktop (please complete the following information): n/a, this occurs in CI

Additional context

dhruvesh commented 2 months ago

I am facing same issue: https://github.com/Azure/azure-rest-api-specs/pull/29961

sdorbala commented 2 months ago

I'm facing the same issue on my PR https://github.com/Azure/azure-rest-api-specs-pr/pull/19036

arjun-d-patel commented 2 months ago

Same issue in: https://github.com/Azure/azure-rest-api-specs/pull/29998 and https://github.com/Azure/azure-rest-api-specs-pr/pull/18922

pjhanwar commented 2 months ago

Facing same issue in https://github.com/Azure/azure-rest-api-specs-pr/pull/18985

JoseLimaIVD commented 2 months ago

Facing the same issue in https://github.com/Azure/azure-rest-api-specs-pr/pull/18871

rosychen427 commented 1 month ago

facing the same issue: https://github.com/Azure/azure-rest-api-specs-pr/pull/19088

arjun-d-patel commented 1 month ago

Same issue here: https://github.com/Azure/azure-rest-api-specs/pull/30271

priyankarking commented 1 month ago

I am facing same issue https://github.com/Azure/azure-rest-api-specs/pull/30293

priyankarking commented 1 month ago

how to suppress this issue or make it optional ?

mikeharder commented 1 week ago

Fixed by #729

mikeharder commented 1 week ago

Fix #729 requires the path after the last . to contain an even number of path segments:

https://github.com/Azure/azure-openapi-validator/blob/c26c69a9a4bc2c36a98752bb7dd89a7292e4a39a/packages/rulesets/src/native/utilities/rules-helper.ts#L140-L152

Which fixed cases like this:

https://github.com/Azure/azure-rest-api-specs-pr/blob/b928ddf9ff0b8a721fe51b8694276e2a864e2982/specification/edge/resource-manager/Microsoft.Edge/configurationmanager/preview/2024-06-01-preview/configurationmanager.json#L322

However, we have an example of a spec with an odd number of path segments instead:

https://github.com/Azure/azure-rest-api-specs-pr/pull/19621/files#diff-25d510d5d4b2927e4a5e839a6258baf6c9f7be821f48e3b7602383fb67647acfR125

Should the rule be fixed to allow this path? Or is the rule correct, and the spec should be updated to use an even number of path segments?