feat(apim): Add new tracing API #23800
Merged
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
|
Hi, @Mielek Thanks for your PR. I am workflow bot for review process. Here are some small tips. Any feedback about review process or workflow bot, pls contact swagger and tools team. [email protected] |
Swagger Validation Report
|
| compared swaggers (via Oad v0.10.4)] | new version | base version |
|---|---|---|
| apimgateways.json | 2023-03-01-preview(547cff4) | 2022-08-01(main) |
| apimgateways.json | 2023-03-01-preview(547cff4) | 2022-09-01-preview(main) |
| definitions.json | 2023-03-01-preview(547cff4) | 2022-08-01(main) |
| definitions.json | 2023-03-01-preview(547cff4) | 2022-09-01-preview(main) |
️⚠️LintDiff: 2 Warnings warning [Detail]
| compared tags (via openapi-validator v2.1.1) | new version | base version |
|---|---|---|
| package-preview-2023-03 | package-preview-2023-03(547cff4) | package-preview-2023-03(release-Microsoft.ApiManagement-2023-03-01-preview) |
[must fix]The following errors/warnings are introduced by current PR:
| Rule | Message | Related RPC [For API reviewers] |
|---|---|---|
| Parameter should have a description. Location: Microsoft.ApiManagement/preview/2023-03-01-preview/apimgateways.json#L1392 |
||
| Parameter should have a description. Location: Microsoft.ApiManagement/preview/2023-03-01-preview/apimgateways.json#L1445 |
The following errors/warnings exist before current PR submission:
Only 30 items are listed, please refer to log for more details.
| Rule | Message |
|---|---|
|
Get endpoints for collections of resources must only have the value and nextLink properties in their model.Location: Microsoft.ApiManagement/preview/2023-03-01-preview/apimgateways.json#L79 |
|
Description of 201 response code of a PUT operation MUST include term 'create'. Location: Microsoft.ApiManagement/preview/2023-03-01-preview/apimgateways.json#L231 |
|
The patch operation body parameter schema should not contains property name. Location: Microsoft.ApiManagement/preview/2023-03-01-preview/apimgateways.json#L285 |
|
The patch operation body parameter schema should not contains property type. Location: Microsoft.ApiManagement/preview/2023-03-01-preview/apimgateways.json#L285 |
|
Get endpoints for collections of resources must only have the value and nextLink properties in their model.Location: Microsoft.ApiManagement/preview/2023-03-01-preview/apimgateways.json#L859 |
|
A PUT operation request body schema should be the same as its 200 response schema, to allow reusing the same entity between GET and PUT. If the schema of the PUT request body is a superset of the GET response body, make sure you have a PATCH operation to make the resource updatable. Operation: 'GatewayApi_CreateOrUpdate' Request Model: 'parameters[6].schema' Response Model: 'responses[200].schema' Location: Microsoft.ApiManagement/preview/2023-03-01-preview/apimgateways.json#L926 |
|
Description of 200 response code of a PUT operation MUST include term 'update'. Location: Microsoft.ApiManagement/preview/2023-03-01-preview/apimgateways.json#L965 |
|
Description of 201 response code of a PUT operation MUST include term 'create'. Location: Microsoft.ApiManagement/preview/2023-03-01-preview/apimgateways.json#L965 |
|
Description of 201 response code of a PUT operation MUST include term 'create'. Location: Microsoft.ApiManagement/preview/2023-03-01-preview/apimgateways.json#L1239 |
|
'items.type' property must be truthy Location: Microsoft.ApiManagement/preview/2023-03-01-preview/definitions.json#L4706 |
|
ProvisioningState must have terminal states: Succeeded, Failed and Canceled. Location: Microsoft.ApiManagement/preview/2023-03-01-preview/definitions.json#L7693 |
|
Property name should be camel case. Location: Microsoft.ApiManagement/preview/2023-03-01-preview/definitions.json#L8132 |
| Parameter should have a description. Location: Microsoft.ApiManagement/preview/2023-03-01-preview/apimgateways.json#L213 |
|
| Parameter should have a description. Location: Microsoft.ApiManagement/preview/2023-03-01-preview/apimgateways.json#L285 |
|
| Parameter should have a description. Location: Microsoft.ApiManagement/preview/2023-03-01-preview/apimgateways.json#L444 |
|
| Parameter should have a description. Location: Microsoft.ApiManagement/preview/2023-03-01-preview/apimgateways.json#L494 |
|
| Parameter should have a description. Location: Microsoft.ApiManagement/preview/2023-03-01-preview/apimgateways.json#L716 |
|
| Parameter should have a description. Location: Microsoft.ApiManagement/preview/2023-03-01-preview/apimgateways.json#L956 |
|
| Parameter should have a description. Location: Microsoft.ApiManagement/preview/2023-03-01-preview/apimgateways.json#L1224 |
|
| Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum. Location: Microsoft.ApiManagement/preview/2023-03-01-preview/definitions.json#L89 |
|
| Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum. Location: Microsoft.ApiManagement/preview/2023-03-01-preview/definitions.json#L117 |
|
| Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum. Location: Microsoft.ApiManagement/preview/2023-03-01-preview/definitions.json#L152 |
|
| Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum. Location: Microsoft.ApiManagement/preview/2023-03-01-preview/definitions.json#L173 |
|
| Consider using x-ms-client-flatten to provide a better end user experience Location: Microsoft.ApiManagement/preview/2023-03-01-preview/definitions.json#L454 |
|
| Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum. Location: Microsoft.ApiManagement/preview/2023-03-01-preview/definitions.json#L579 |
|
| Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum. Location: Microsoft.ApiManagement/preview/2023-03-01-preview/definitions.json#L583 |
|
| Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum. Location: Microsoft.ApiManagement/preview/2023-03-01-preview/definitions.json#L602 |
|
| Consider using x-ms-client-flatten to provide a better end user experience Location: Microsoft.ApiManagement/preview/2023-03-01-preview/definitions.json#L697 |
|
| Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum. Location: Microsoft.ApiManagement/preview/2023-03-01-preview/definitions.json#L841 |
|
| Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum. Location: Microsoft.ApiManagement/preview/2023-03-01-preview/definitions.json#L846 |
️️✔️Avocado succeeded [Detail] [Expand]
Validation passes for Avocado.
️️✔️SwaggerAPIView succeeded [Detail] [Expand]
️️✔️CadlAPIView succeeded [Detail] [Expand]
️️✔️TypeSpecAPIView succeeded [Detail] [Expand]
️️✔️ModelValidation succeeded [Detail] [Expand]
Validation passes for ModelValidation.
️️✔️SemanticValidation succeeded [Detail] [Expand]
Validation passes for SemanticValidation.
️️✔️PrettierCheck succeeded [Detail] [Expand]
Validation passes for PrettierCheck.
️️✔️SpellCheck succeeded [Detail] [Expand]
Validation passes for SpellCheck.
️️✔️CadlValidation succeeded [Detail] [Expand]
Validation passes for CadlValidation.
️️✔️TypeSpec Validation succeeded [Detail] [Expand]
Validation passes for TypeSpec Validation.
️️✔️PR Summary succeeded [Detail] [Expand]
Validation passes for Summary.
Swagger Generation Artifacts
|
Generated ApiView
|
ghost
added
the
Mielek
changed the title
Mielek
changed the title
|
@Mielek please fix the issues with
|
AzureRestAPISpecReview
added
ARMReview
resource-manager
WaitForARMFeedback
|
@Mielek - I can remove the tag, do you want an arm service review now? |
|
@zedy-wj yes, if you can remove the tag and allow arm service review it would be great. |
zedy-wj
removed
the
ARMChangesRequested
openapi-workflow-bot
bot
added
the
WaitForARMFeedback
|
note: this looks like adding new operations to an existing public preview API. I think that's OK to be done with a new api-version as long as its a nonbreaking change. |
...agement/resource-manager/Microsoft.ApiManagement/preview/2023-03-01-preview/definitions.json
Show resolved
Hide resolved
...agement/resource-manager/Microsoft.ApiManagement/preview/2023-03-01-preview/definitions.json
Show resolved
Hide resolved
...agement/resource-manager/Microsoft.ApiManagement/preview/2023-03-01-preview/definitions.json
Outdated
Show resolved
Hide resolved
| } | ||
| ] | ||
| } | ||
| } |
There was a problem hiding this comment.
Looks like there is potential to return sensitive data via uris, headers, etc.
Is there anything you would be planning to do to allow people to avoid tracing and then accidentally disclosing, sensitive information in their traces?
There was a problem hiding this comment.
@TimLovellSmith This is a debug functionality. It targets administrators of system to be able to check how request was processed and find if there is any error in that process. To create trace, customer needs to generate debug credentials. The credentials need to be send in the header with request which goes through APIM gateway proxy. This will allow customer to create trace. Because customer needs to specially craft the request, all possible sensitive data, customer should already know.
The functionality was available in APIM before. Now we want to secure it by applying RBAC security.
@VitaliyKurokhtin Can you please expand on the idea if I am missing something.
There was a problem hiding this comment.
Yes, this whole API is actually a step towards more secure tracing functionality. At the moment we're solving problem of separating ability to make request, ability to request trace and ability to view resulting trace via separate API actions to allow for granular RBAC control. The second phase of changes will include stripping knowingly secret information from traces - this is not at all trivial given the flexibility we allow for how this information may be manipulated. The third phase would result in inclusion of heuristic tools that would strip away anything that looks like a secret. Since actual content of the trace is left not specified on purpose phases two and three would be non-breaking for customers.
TimLovellSmith
added
the
ARMChangesRequested
openapi-workflow-bot
bot
removed
the
WaitForARMFeedback
|
Please respond to provided suggestions and then remove ArmChangesRequested label. |
@TimLovellSmith this a new preview api-version we are collecting changes for. Look at the destination branch for this PR. Once all individual changes are checked in, we then merge to You can find all the PRs for this api-version 2023-03-01-preview-apim |
|
@solankisamir or @zedy-wj can one of you please remove the |
zedy-wj
removed
the
ARMChangesRequested
openapi-workflow-bot
bot
added
the
WaitForARMFeedback
TimLovellSmith
added
the
ARMSignedOff
openapi-workflow-bot
bot
removed
the
WaitForARMFeedback
|
@zedy-wj can you please a second look at this one? Looks like it is waiting on your review and final merge. |
zedy-wj
approved these changes
May 15, 2023
zedy-wj
added
the
Approved-OkToMerge
|
@Mielek, @solankisamir - When are you going to merge? |
|
@zedy-wj it is funny but again I am not able to do it because I am not authorized. @solankisamir Please can you merge it? |
|
Only @zedy-wj can merge it. Please go ahead, we are good! |
live1206
merged commit 3637c60
into
Azure:release-Microsoft.ApiManagement-2023-03-01-preview
live1206
pushed a commit
that referenced
this pull request
Jun 25, 2023
* Adds base for updating Microsoft.ApiManagement from version preview/2022-09-01-preview to version 2023-03-01-preview * Updates readme * Updates API version in new specs and examples * feat(apim): Provide capability to use custom hostname for Configuration API v2 (#23785) Signed-off-by: Tom Kerkhove <[email protected]> * feat(rp): Added flags to enable/disable portals (#23799) * feat(rp): Added flags to enable/disable portals * update default * feat(apim): Provide capability to disable legacy configuration API (#23786) * feat(apim): Provide capability to disable legacy configuration API Signed-off-by: Tom Kerkhove <[email protected]> * Update sample Signed-off-by: Tom Kerkhove <[email protected]> * Fix type definition Signed-off-by: Tom Kerkhove <[email protected]> * Switch to enum Signed-off-by: Tom Kerkhove <[email protected]> * Update samples Signed-off-by: Tom Kerkhove <[email protected]> --------- Signed-off-by: Tom Kerkhove <[email protected]> * feat(apim): Add new tracing API (#23800) * Add new tracing api * Remove TODOs descriptions * Improve descriptions * Fix dot * Add missing examples * prettier fixes * Fix model and semantic problems * Remove get prefix in post operations * Use list prefix * Fix PR comments * feat(apim): Add ability to create and update new OData api (#23849) * Add odata api definitions * add api usage example * fix file reference * address comments --------- Co-authored-by: rkolesnikov <[email protected]> * migration contract (#23983) * 2023 03 cred scan fix - Examples Only (#24357) * remove-sas-signature * update example * 2023-03-01-preview arm review feedback (#24404) * move to v5 subscription contract * move to v5 for rg and api-version * cred scan * default is true * uuid in examples * change from boolean to enums * fix default value --------- Signed-off-by: Tom Kerkhove <[email protected]> Co-authored-by: Tom Kerkhove <[email protected]> Co-authored-by: Harun Reşit <[email protected]> Co-authored-by: Rafał Mielowski <[email protected]> Co-authored-by: Roman Kolesnikov <[email protected]> Co-authored-by: rkolesnikov <[email protected]>
harryli0108
pushed a commit
to harryli0108/azure-rest-api-specs
that referenced
this pull request
Jul 28, 2023
* Adds base for updating Microsoft.ApiManagement from version preview/2022-09-01-preview to version 2023-03-01-preview * Updates readme * Updates API version in new specs and examples * feat(apim): Provide capability to use custom hostname for Configuration API v2 (Azure#23785) Signed-off-by: Tom Kerkhove <[email protected]> * feat(rp): Added flags to enable/disable portals (Azure#23799) * feat(rp): Added flags to enable/disable portals * update default * feat(apim): Provide capability to disable legacy configuration API (Azure#23786) * feat(apim): Provide capability to disable legacy configuration API Signed-off-by: Tom Kerkhove <[email protected]> * Update sample Signed-off-by: Tom Kerkhove <[email protected]> * Fix type definition Signed-off-by: Tom Kerkhove <[email protected]> * Switch to enum Signed-off-by: Tom Kerkhove <[email protected]> * Update samples Signed-off-by: Tom Kerkhove <[email protected]> --------- Signed-off-by: Tom Kerkhove <[email protected]> * feat(apim): Add new tracing API (Azure#23800) * Add new tracing api * Remove TODOs descriptions * Improve descriptions * Fix dot * Add missing examples * prettier fixes * Fix model and semantic problems * Remove get prefix in post operations * Use list prefix * Fix PR comments * feat(apim): Add ability to create and update new OData api (Azure#23849) * Add odata api definitions * add api usage example * fix file reference * address comments --------- Co-authored-by: rkolesnikov <[email protected]> * migration contract (Azure#23983) * 2023 03 cred scan fix - Examples Only (Azure#24357) * remove-sas-signature * update example * 2023-03-01-preview arm review feedback (Azure#24404) * move to v5 subscription contract * move to v5 for rg and api-version * cred scan * default is true * uuid in examples * change from boolean to enums * fix default value --------- Signed-off-by: Tom Kerkhove <[email protected]> Co-authored-by: Tom Kerkhove <[email protected]> Co-authored-by: Harun Reşit <[email protected]> Co-authored-by: Rafał Mielowski <[email protected]> Co-authored-by: Roman Kolesnikov <[email protected]> Co-authored-by: rkolesnikov <[email protected]>
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.
ARM API Information (Control Plane)
MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.
Azure 1st Party Service can try out the Shift Left experience to initiate API design review from ADO code repo. If you are interested, may request engineering support by filling in with the form https://aka.ms/ShiftLeftSupportForm.
Changelog
Add a changelog entry for this PR by answering the following questions:
Contribution checklist (MS Employees Only):
If any further question about AME onboarding or validation tools, please view the FAQ.
ARM API Review Checklist
Otherwise your PR may be subject to ARM review requirements. Complete the following:
Check this box if any of the following apply to the PR so that the label "ARMReview" and "WaitForARMFeedback" will be added by bot to kick off ARM API Review. Missing to check this box in the following scenario may result in delays to the ARM manifest review and deployment.
-[ ] To review changes efficiently, ensure you copy the existing version into the new directory structure for first commit and then push new changes, including version updates, in separate commits. You can use OpenAPIHub to initialize the PR for adding a new version. For more details refer to the wiki.
Ensure you've reviewed following guidelines including ARM resource provider contract and REST guidelines. Estimated time (4 hours). This is required before you can request review from ARM API Review board.
If you are blocked on ARM review and want to get the PR merged with urgency, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.
Breaking Change Review Checklist
If you have any breaking changes as defined in the Breaking Change Policy, request approval from the Breaking Change Review Board.
Action: to initiate an evaluation of the breaking change, create a new intake using the template for breaking changes. Additional details on the process and office hours are on the Breaking Change Wiki.
NOTE: To update API(s) in public preview for over 1 year (refer to Retirement of Previews)
Please follow the link to find more details on PR review process.