Add tests to account for tenant level APIs for all RPC rule for PUT operations #632
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bdefoy
reviewed
Dec 6, 2023
docs/repeated-path-info.md
Outdated
| Information in the URI should not be repeated in the request body (i.e. subscription ID, resource group name, resource name). | ||
|
|
||
| ## CreatedAt | ||
| The '{property name}' already appears in the URI, please don't repeat it in the request body. Information in the URI must not be repeated in the request body (i.e. subscription ID, resource group name, resource name). |
There was a problem hiding this comment.
Suggested change
| The '{property name}' already appears in the URI, please don't repeat it in the request body. Information in the URI must not be repeated in the request body (i.e. subscription ID, resource group name, resource name). | |
| The '{property name}' property is in the URI, so it must not be in the request body. Information in the URI must not be repeated in the request body (i.e., subscription ID, resource group name, resource name). | |
| ``` #Resolved |
bdefoy
reviewed
Dec 6, 2023
|
|
||
|
|
||
| ## Why the rule is important | ||
| A PUT or Patch request must always have a request body defined. This rule applies to all ARM resources (Tracked and proxy). PUT and patch operations using an empty payload is not allowed in ARM. |
There was a problem hiding this comment.
Suggested change
| A PUT or Patch request must always have a request body defined. This rule applies to all ARM resources (Tracked and proxy). PUT and patch operations using an empty payload is not allowed in ARM. | |
| A PUT or PATCH request must always have a request body defined. This rule applies to all ARM resources (Tracked and Proxy). PUT and PATCH operations using an empty payload are not allowed in ARM. | |
| ``` #Resolved |
|
|
||
| ## How to fix the violation | ||
| Add a request body for every PUT or Patch operation defined in your swagger. This request body must also match the response body of the PUT or Patch operation. |
There was a problem hiding this comment.
Suggested change
| Add a request body for every PUT or Patch operation defined in your swagger. This request body must also match the response body of the PUT or Patch operation. | |
| Add a request body for every PUT or PATCH operation defined in your swagger. This request body must also match the response body of the PUT or PATCH operation. | |
| ``` #Resolved |
| ## Bad example 2 | ||
|
|
||
| Put without a request body | ||
| Another Put operation without a request body |
There was a problem hiding this comment.
Suggested change
| Another Put operation without a request body | |
| Another PUT operation without a request body: | |
| ``` #Resolved |
| ## Bad example 1 | ||
|
|
||
| Put without a request body | ||
| PUT without a request body |
There was a problem hiding this comment.
Suggested change
| PUT without a request body | |
| PUT without a request body: | |
| ``` #Resolved |
|
|
||
| ## Bad examples | ||
|
|
||
| ```json |
There was a problem hiding this comment.
I noticed this in some other file, maybe it would remove the syntax error highlighting?
Suggested change
| ```json | |
| ```json5 | |
| ``` #Resolved |
| ## Output Message | ||
|
|
||
| The tracked resource {resourceName} is beyond third level of nesting. | ||
|
|
||
| ## Description | ||
|
|
||
| Tracked resources must not be used beyond the third level of nesting. |
There was a problem hiding this comment.
Should this be beyond the second level of nesting? Meaning tracked is ok for first (parent) and second level (child) resources, but not third level (grandchild). Make sure this is consistent with line 21.
Suggested change
| Tracked resources must not be used beyond the third level of nesting. | |
| Tracked resources must not be used beyond the second level of nesting. | |
| ``` #Resolved |
There was a problem hiding this comment.
Ah the description is inconsistent with the how to fix section. I will fix it.
|
|
||
| Avoid the third level nested tracked resource, you should try to decrease the nested levels. | ||
| Avoid the third level nested tracked resource. You may model such resources as proxy resources instead. Having a tracked resource beyond the third level may lead to a loss of functionality in ARM. |
There was a problem hiding this comment.
Third level is "beyond the second level".
Suggested change
| Avoid the third level nested tracked resource. You may model such resources as proxy resources instead. Having a tracked resource beyond the third level may lead to a loss of functionality in ARM. | |
| Avoid the third level nested tracked resource. You may model such resources as proxy resources instead. Having a tracked resource beyond the second level may lead to a loss of functionality in ARM. | |
| ``` #Resolved |
There was a problem hiding this comment.
Thats correct and thats the reason the third level must be avoided.
There was a problem hiding this comment.
Ah the description is inconsistent with the how to fix section. I will fix it.
docs/tracked-resource-schema-tags.md
Outdated
| ## Why the rule is important | ||
|
|
||
| ARM requires all tracked resources to support taggability. | ||
| Every tracked resource MUST support tags as an optional property. The specified tracked resource either does not have tags mentioned as a property or it is mentioned but marked as required. |
There was a problem hiding this comment.
Suggested change
| Every tracked resource MUST support tags as an optional property. The specified tracked resource either does not have tags mentioned as a property or it is mentioned but marked as required. | |
| Every tracked resource **must** support `tags` as an **optional** property. The specified tracked resource either does not have `tags` as a property or has `tags` marked as required. | |
| ``` #Resolved |
|
|
||
| Adding the put operation for tracked resource, or remove the resource definition if it's useless. | ||
| Add the PUT operation for a tracked resource or make the resource proxy if it cannot be created by the end user. |
There was a problem hiding this comment.
Suggested change
| Add the PUT operation for a tracked resource or make the resource proxy if it cannot be created by the end user. | |
| Add a PUT operation for the tracked resource or make the resource's routing type proxy if it cannot be created by the end user. | |
| ``` #Resolved |
bdefoy
approved these changes
Jan 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is to ensure coverage of all the RC rules for PUT operations for tenant level APIs. No changes to the rules were necessary and a bunch of tests were added to test for coverage wherever relevant.