azure-rest-api-specs icon indicating copy to clipboard operation
azure-rest-api-specs copied to clipboard

Nipadhye/firewall packet capture feature

Open nikhilpadhye1 opened this issue 3 years ago • 16 comments

ARM API Information (Control Plane)

** Closed another PR which was active to move the feature to the correct API Version** Link to closed PR: https://github.com/Azure/azure-rest-api-specs/pull/20601 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:

  1. What's the purpose of the update?
    • [ ] new service onboarding
    • [ ] new API version
    • [X] update existing version for new feature
    • [ ] update existing version to fix swagger quality issue in s360
    • [ ] Other, please clarify
  2. When are you targeting to deploy the new service/feature to public regions? Please provide the date or, if the date is not yet available, the month. NRP 170 - 9/23
  3. When do you expect to publish the swagger? Please provide date or, the the date is not yet available, the month. Next release, 9/23 NRP 170 release
  4. By default, Azure SDKs of all languages (.NET/Python/Java/JavaScript for both management-plane SDK and data-plane SDK, Go for management-plane SDK only ) MUST be refreshed with/after swagger of new version is published. If you prefer NOT to refresh any specific SDK language upon swagger updates in the current PR, please leave details with justification here.

Contribution checklist (MS Employees Only):

If any further question about AME onboarding or validation tools, please view the FAQ.

ARM API Review Checklist

Applicability: :warning:

If your changes encompass only the following scenarios, you should SKIP this section, as these scenarios do not require ARM review.

  • Change to data plane APIs
  • Adding new properties
  • All removals

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.

    • Adding a new service
    • Adding new API(s)
    • Adding a new API version -[ ] 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.

nikhilpadhye1 avatar Oct 12 '22 08:10 nikhilpadhye1

Hi, @nikhilpadhye1 Thanks for your PR. I am workflow bot for review process. Here are some small tips.

  • Please ensure to do self-check against checklists in first PR comment.
  • PR assignee is the person auto-assigned and responsible for your current PR reviewing and merging.
  • For specs comparison cross API versions, Use API Specs Comparison Report Generator
  • If there is CI failure(s), to fix CI error(s) is mandatory for PR merging; or you need to provide justification in PR comment for explanation. How to fix?
  • Any feedback about review process or workflow bot, pls contact swagger and tools team. [email protected]

    Swagger Validation Report

    ️❌BreakingChange: 1005 Errors, 11 Warnings failed [Detail]

    Only 0 items are rendered, please refer to log for more details.

    Only -1 items are listed, please refer to log for more details.

    Rule Message
    ️❌Breaking Change(Cross-Version): 21 Errors, 26 Warnings failed [Detail]

    Only 0 items are rendered, please refer to log for more details.

    The following breaking changes are detected by comparison with the latest stable version:

    Only -1 items are listed, please refer to log for more details.

    Rule Message
    ️🔄LintDiff inProgress [Detail]
    ️❌Avocado: 761 Errors, 5 Warnings failed [Detail]

    Only -1 items are listed, please refer to log for more details.

    Rule Message
    ️❌SwaggerAPIView: 0 Errors, 0 Warnings failed [Detail]
    ️🔄CadlAPIView inProgress [Detail]
    ️❌ModelValidation: 800 Errors, 0 Warnings failed [Detail]

    Only -1 items are listed, please refer to log for more details.

    Rule Message
    ️❌SemanticValidation: 9 Errors, 0 Warnings failed [Detail]

    Only -1 items are listed, please refer to log for more details.

    Rule Message
    ️❌PrettierCheck: 1 Errors, 0 Warnings failed [Detail]

    Only -1 items are listed, please refer to log for more details.

    Rule Message
    ️️✔️SpellCheck succeeded [Detail] [Expand]
    Validation passes for SpellCheck.
    ️❌CadlValidation: 12 Errors, 11 Warnings failed [Detail]

    Only -1 items are listed, please refer to log for more details.

    Rule Message
    ️️✔️PR Summary succeeded [Detail] [Expand]
    Validation passes for Summary.
    Posted by Swagger Pipeline | How to fix these errors?

    Swagger Generation Artifacts

    ️️✔️ApiDocPreview succeeded [Detail] [Expand]

    Only 0 items are rendered, please refer to log for more details.

    ️⚠️SDK Breaking Change Tracking warning [Detail]

    Only 0 items are rendered, please refer to log for more details.

    ️❌ azure-sdk-for-net-track2 failed [Detail]

    Only 0 items are rendered, please refer to log for more details.

    ️❌ azure-sdk-for-python failed [Detail]

    Only 0 items are rendered, please refer to log for more details.

    ️❌ azure-powershell failed [Detail]

    Only 0 items are rendered, please refer to log for more details.

    ️🔄 azure-sdk-for-python-track2 inProgress [Detail]
    ️⚠️ azure-sdk-for-java warning [Detail]

    Only 0 items are rendered, please refer to log for more details.

    ️🔄 azure-sdk-for-js inProgress [Detail]
    ️️✔️ azure-sdk-for-go succeeded [Detail] [Expand]

    Only 0 items are rendered, please refer to log for more details.

    ️🔄 azure-resource-manager-schemas inProgress [Detail]
    ️❌ azure-sdk-for-net failed [Detail]

    Only 0 items are rendered, please refer to log for more details.

    Posted by Swagger Pipeline | How to fix these errors?

    Generated ApiView

    Language Package Name ApiView Link
    Go sdk/resourcemanager/appplatform/armappplatform https://apiview.dev/Assemblies/Review/5d8d534fa7fe43bfbe474e4c45f2e734
    Go sdk/resourcemanager/azurestackhci/armazurestackhci https://apiview.dev/Assemblies/Review/d87632f36deb4e8fa49d47dc85ce97f3
    Go sdk/resourcemanager/billingbenefits/armbillingbenefits https://apiview.dev/Assemblies/Review/298510e2726e44bba1238d9265f269de
    Go sdk/resourcemanager/cdn/armcdn https://apiview.dev/Assemblies/Review/eca21e21e61948cbac425c18c1d47e7a
    Go sdk/resourcemanager/resources/armchanges https://apiview.dev/Assemblies/Review/41260014c5e84153909c2ff10ad40702
    Go sdk/resourcemanager/chaos/armchaos https://apiview.dev/Assemblies/Review/48a44a15051946b0951199cedf59d345
    Go sdk/resourcemanager/connectedvmware/armconnectedvmware https://apiview.dev/Assemblies/Review/703c4472df6c4631850fffdc3271251d
    Go sdk/resourcemanager/consumption/armconsumption https://apiview.dev/Assemblies/Review/6d3ae1f7a0ed4aafab9195725cb082b5
    Go sdk/resourcemanager/containerinstance/armcontainerinstance https://apiview.dev/Assemblies/Review/c5a1535402994b64bb1d8f323819466a
    Go sdk/resourcemanager/containerregistry/armcontainerregistry https://apiview.dev/Assemblies/Review/e7f5b5b317474861a6605ef3a2d8067e
    Go sdk/resourcemanager/containerservice/armcontainerservice https://apiview.dev/Assemblies/Review/0309b0fbaaf14b9a86080b0495a111c1
    Go sdk/resourcemanager/costmanagement/armcostmanagement https://apiview.dev/Assemblies/Review/b2d23d3037a148159178509ad9cbfc28
    Go sdk/resourcemanager/databox/armdatabox https://apiview.dev/Assemblies/Review/7b732265c37c4d3fb51b036b04c7182e
    Go sdk/resourcemanager/dataprotection/armdataprotection https://apiview.dev/Assemblies/Review/3fd43f51c1fe45758ad1d27875275d04
    Go sdk/resourcemanager/resources/armdeploymentscripts https://apiview.dev/Assemblies/Review/0be23d0502254a06a5e34de1ecc32c9b
    Go sdk/resourcemanager/devcenter/armdevcenter https://apiview.dev/Assemblies/Review/27a7b7e93f3b4adca34bf3eb82d1672f
    Go sdk/resourcemanager/digitaltwins/armdigitaltwins https://apiview.dev/Assemblies/Review/10825a976afa4a21aca0a53e7b178e0f
    Go sdk/resourcemanager/domainservices/armdomainservices https://apiview.dev/Assemblies/Review/803feba7eaeb49f5865ccd0cf3aa27dc
    Go sdk/resourcemanager/eventhub/armeventhub https://apiview.dev/Assemblies/Review/ec600a6cec4a45c69537b691340db178
    Go sdk/resourcemanager/resources/armfeatures https://apiview.dev/Assemblies/Review/a1cc37ade6424f1583a03e9edce89f64
    Go sdk/resourcemanager/healthcareapis/armhealthcareapis https://apiview.dev/Assemblies/Review/efd93fee7c2a4c199cef21df8474a261
    Go sdk/resourcemanager/resources/armlinks https://apiview.dev/Assemblies/Review/1e54b240d0d546118d2138cd50e0f0c4
    Go sdk/resourcemanager/loadtesting/armloadtesting There is no API change compared with the previous version
    Go sdk/resourcemanager/resources/armlocks https://apiview.dev/Assemblies/Review/ea865edfb61e4e63910e69cc946546d0
    Go sdk/resourcemanager/maintenance/armmaintenance https://apiview.dev/Assemblies/Review/e2afcfb16d9f4b93b4b74b6d9a6740df
    Go sdk/resourcemanager/resources/armmanagedapplications https://apiview.dev/Assemblies/Review/a3a1415cc0b54f2fbb17fc8bb8d4599a
    Go sdk/resourcemanager/network/armnetwork https://apiview.dev/Assemblies/Review/59286f1e258441448d22c9187958f965
    Go sdk/resourcemanager/networkfunction/armnetworkfunction https://apiview.dev/Assemblies/Review/0a5eef597fa64e688760584e6d5bfc92
    Go sdk/resourcemanager/resources/armpolicy https://apiview.dev/Assemblies/Review/15ce19c9c4f64bc0bf40795c7785a766
    Go sdk/resourcemanager/recoveryservices/armrecoveryservices https://apiview.dev/Assemblies/Review/6c3b28df82c147a49a9ad7ad61d7eb62
    Go sdk/resourcemanager/recoveryservices/armrecoveryservicesbackup https://apiview.dev/Assemblies/Review/71abb50d08904b3cb1801d3d0344a878
    Go sdk/resourcemanager/recoveryservices/armrecoveryservicessiterecovery https://apiview.dev/Assemblies/Review/b376936d8f3743bbba4af9e20a53a95f
    Go sdk/resourcemanager/resources/armresources https://apiview.dev/Assemblies/Review/5c8389dde80e422c93c722f0de5e3c4a
    Go sdk/resourcemanager/security/armsecurity https://apiview.dev/Assemblies/Review/739afd4eff4a4c16a6793d43ab51117b
    Go sdk/resourcemanager/servicelinker/armservicelinker https://apiview.dev/Assemblies/Review/bb371d3df02d42a08655c57e700b17cd
    Go sdk/resourcemanager/sqlvirtualmachine/armsqlvirtualmachine https://apiview.dev/Assemblies/Review/73205fb40716401382b209aab7754909
    Go sdk/resourcemanager/streamanalytics/armstreamanalytics https://apiview.dev/Assemblies/Review/76541eb5518345c18e9cdf21a0cbd8ec
    Go sdk/resourcemanager/resources/armsubscriptions https://apiview.dev/Assemblies/Review/a47e6e6aa1a746e681dd5f0531b17594
    Go sdk/resourcemanager/synapse/armsynapse https://apiview.dev/Assemblies/Review/fd773f6a94e349ddb50326d7d101fea2
    Go sdk/resourcemanager/resources/armtemplatespecs https://apiview.dev/Assemblies/Review/922f4f50ac0c4f40bde40ed7b16ba9a1
    Go sdk/resourcemanager/workloads/armworkloads https://apiview.dev/Assemblies/Review/f1ee6e25e45a4c3d8a1cf474697de7a1
    Java azure-resourcemanager-appplatform-generated https://apiview.dev/Assemblies/Review/359ede66db324eae8b9fc61c32a47183
    Java azure-resourcemanager-billingbenefits https://apiview.dev/Assemblies/Review/d54a4e1ff0924556b91284b212428015
    Java azure-resourcemanager-cdn-generated https://apiview.dev/Assemblies/Review/e047fa248dab41bd983d910690f94848
    Java azure-resourcemanager-chaos https://apiview.dev/Assemblies/Review/fafb045ee68b4f35abb5c5715ed144b8
    Java azure-resourcemanager-connectedvmware https://apiview.dev/Assemblies/Review/b3564519fc80484db3c37875beadcfb0
    Java azure-resourcemanager-consumption https://apiview.dev/Assemblies/Review/43e5cb0a0a7d4750b9e8c543cda5e5fc
    Java azure-resourcemanager-containerinstance-generated https://apiview.dev/Assemblies/Review/e464c4ef18684c54ad9d62da1c9082cc
    Java azure-resourcemanager-containerregistry-generated https://apiview.dev/Assemblies/Review/d7f8ec0d5aed4cbabce8019a512fb014
    Java azure-resourcemanager-containerservice-generated https://apiview.dev/Assemblies/Review/fbbb66d073f94542943464e73998fb98
    Java azure-resourcemanager-databox https://apiview.dev/Assemblies/Review/05bcbc0d64904f928953a56e233d1ecc
    Java azure-resourcemanager-dataprotection https://apiview.dev/Assemblies/Review/8f79fb1e71da450a865fc11fef55e25d
    Java azure-resourcemanager-devcenter https://apiview.dev/Assemblies/Review/7a2691480d874e42805e84b03673f4fc
    Java azure-resourcemanager-digitaltwins https://apiview.dev/Assemblies/Review/a7146813a0da4e688f0a5da95d740bbf
    Java azure-resourcemanager-eventhubs-generated https://apiview.dev/Assemblies/Review/c1fe0fba0751488db81019c001ed1f1c
    Java azure-resourcemanager-healthcareapis https://apiview.dev/Assemblies/Review/0c3e04240f2a475dbb82498f9793df84
    Java azure-resourcemanager-liftrqumulo https://apiview.dev/Assemblies/Review/77af8d95841a43338e0f7b52d50a8ea6
    Java azure-resourcemanager-loadtesting There is no API change compared with the previous version
    Java azure-resourcemanager-network-generated https://apiview.dev/Assemblies/Review/c1b35d9bab39440ba00193edf9c48a26
    Java azure-resourcemanager-networkfunction https://apiview.dev/Assemblies/Review/e9df7b34055147e88db455ea698742a3
    Java azure-resourcemanager-recoveryservices There is no API change compared with the previous version
    Java azure-resourcemanager-recoveryservicesbackup https://apiview.dev/Assemblies/Review/c6306ad2f4284a5fb9e9deeb8cef2ec2
    Java azure-resourcemanager-recoveryservicessiterecovery https://apiview.dev/Assemblies/Review/5e73800f18604ce0bf9a7aeb6847e87b
    Java azure-resourcemanager-resourcehealth https://apiview.dev/Assemblies/Review/a6ef644cb3b2434bbccd7b03f4bbed77
    Java azure-resourcemanager-resources-generated https://apiview.dev/Assemblies/Review/b13aedc4a73d4dd383ac4d192e0934d5
    Java azure-resourcemanager-security https://apiview.dev/Assemblies/Review/bac6424080064fdab6f784caf9be85bb
    Java azure-resourcemanager-securityinsights https://apiview.dev/Assemblies/Review/45a4afd2d43d4cc98f9b1178aba4ef35
    Java azure-resourcemanager-sqlvirtualmachine https://apiview.dev/Assemblies/Review/6ddca9384ecd4182add2c1a7b7e88bb0
    Java azure-resourcemanager-synapse https://apiview.dev/Assemblies/Review/acd01abbbaa94b098aff8b148d56a701
    Java azure-resourcemanager-workloads https://apiview.dev/Assemblies/Review/2d4d6b9aff8a440988aead5ed6be46f1

    @raosuhas @lirenhe moved the PR to correct API version. Ran into issues trying to rebase previous PR branch so making this new active PR. Addressed all the comments left by @raosuhas in the older PR

    nikhilpadhye1 avatar Oct 12 '22 08:10 nikhilpadhye1

    Hi @nikhilpadhye1, Your PR has some issues. Please fix the CI sequentially by following the order of Avocado, semantic validation, model validation, breaking change, lintDiff. If you have any questions, please post your questions in this channel https://aka.ms/swaggersupport.

    TaskHow to fixPriority
    AvocadoFix-AvocadoHigh
    Semantic validationFix-SemanticValidation-ErrorHigh
    Model validationFix-ModelValidation-ErrorHigh
    LintDiffFix-LintDiffhigh
    If you need further help, please feedback via swagger feedback.

    @nikhilpadhye1, could you fix the model validation errors? The Swagger example has some conflicts with API definition.

    lirenhe avatar Oct 13 '22 09:10 lirenhe

    @nikhilpadhye1 Please merge public main branch into Azure:release-network-Microsoft.Network-2022-07-01 to pass python pipeline

    msyyc avatar Oct 14 '22 02:10 msyyc

    Please ensure to respond feedbacks from the ARM API reviewer. When you are ready to continue the ARM API review, please remove ARMChangesRequested

    @nikhilpadhye1 Please merge public main branch into Azure:release-network-Microsoft.Network-2022-07-01 to pass python pipeline

    I tried to merge public main into the release branch and got up-to-date message from command-line @msyyc

    nikhilpadhye1 avatar Oct 14 '22 18:10 nikhilpadhye1

    @lirenhe @raosuhas is there anymore feedback for the PR? Can we close this?

    nikhilpadhye1 avatar Oct 17 '22 19:10 nikhilpadhye1

    @MikhailTryakhov @raosuhas @lirenhe @raych1 Can we please get the review on the PR?

    saisujithreddym avatar Nov 03 '22 22:11 saisujithreddym

    @nikhilpadhye1, Please work with ARM team to address their feedback. I also see the PR to merge from network 2022-07-01 to main branch was already created. Could you work with @MikhailTryakhov about the timeline for this one?

    lirenhe avatar Nov 04 '22 01:11 lirenhe

    Hi, @nikhilpadhye1. Your PR has no update for 14 days and it is marked as stale PR. If no further update for over 14 days, the bot will close the PR. If you want to refresh the PR, please remove no-recent-activity label.

    ghost avatar Nov 27 '22 16:11 ghost

    Hi, @nikhilpadhye1. The PR will be closed since the PR has no update for 28 days. If you still need the PR review to proceed, please reopen it and @ mention PR assignee.

    ghost avatar Dec 12 '22 05:12 ghost

    NewApiVersionRequired reason: A service’s API is a contract with customers and is represented by using the api-version query parameter. Changes such as adding an optional property to a request/response or introducing a new operation is a change to the service’s contract and therefore requires a new api-version value. This is critically important for documentation, client libraries, and customer support. EXAMPLE: if a customer calls a service in the public cloud using api-version=2020-07-27, the new property or operation may exist but if they call the service in a government cloud, air-gapped cloud, or Azure Stack Hub cloud using the same api-version, the property or operation may not exist. Because there is no clear relationship between the service api-version and the new property/operation, customers can’t trust the documentation and Azure customer have difficulty helping customers diagnose issues. In addition, each client library version documents the service version it supports. When an optional property or new operation is added to a service and its Swagger, new client libraries must be produced to expose this functionality to customers. Without updating the api-version, it is unclear to customers which version of a client library supports these new features.

    @raosuhas can you please take a look at the response to comments and fix of NIT and approve or advice on further changes?

    nikhilpadhye1 avatar Jan 31 '23 18:01 nikhilpadhye1

    Hi, @nikhilpadhye1 your PR are labelled with WaitForARMFeedback. A notification email will be sent out shortly afterwards to notify ARM review board([email protected]).

    Please check the lint error in https://github.com/Azure/azure-rest-api-specs/pull/21080/checks?check_run_id=11211115269

    zizw123 avatar Feb 10 '23 22:02 zizw123

    Please check the lint error in https://github.com/Azure/azure-rest-api-specs/pull/21080/checks?check_run_id=11211115269

    Adding the pattern linter error is existing for firewall. Adding the pattern may cause the feature to not be accessible to existing azure firewall customers. If you have a suggestion on how to safely circumvent this possibility, please let me know. Need a solution as this is a blocker for the PR. @zizw123

    nikhilpadhye1 avatar Feb 11 '23 11:02 nikhilpadhye1

    @nikhilpadhye1, I already see the PR to merge NRP 2022-09-01 to main branch. Please work with @MikhailTryakhov if this PR is needed for 2022-09-01, otherwise, you need to wait for the next release.

    lirenhe avatar Feb 17 '23 03:02 lirenhe

    Hi @nikhilpadhye1, one or multiple breaking change(s) is detected in your PR. Please check out the breaking change(s), and provide business justification in the PR comment and @ PR assignee why you must have these change(s), and how external customer impact can be mitigated. Please ensure to follow breaking change policy to request breaking change review and approval before proceeding swagger PR review. Action: To initiate an evaluation of the breaking change, create a new intake using the template for breaking changes. Addition details on the process and office hours are on the Breaking change Wiki. If you want to know the production traffic statistic, please see ARM Traffic statistic. If you think it is false positive breaking change, please provide the reasons in the PR comment, report to Swagger Tooling Team via https://aka.ms/swaggerfeedback. Note: To avoid breaking change, you can refer to Shift Left Solution for detecting breaking change in early phase at your service code repository.

    Hi, @nikhilpadhye1, For review efficiency consideration, when creating a new api version, it is required to place API specs of the base version in the first commit, and push new version updates into successive commits. You can use OpenAPIHub to initialize the PR for adding a new version. For more details refer to the wiki. Or you could onboard API spec pipeline

    Hi @nikhilpadhye1, one or multiple validation error/warning suppression(s) is detected in your PR. Please follow the Swagger-Suppression-Process to get approval.

    /azp run

    raych1 avatar Mar 13 '23 01:03 raych1

    Azure Pipelines successfully started running 1 pipeline(s).

    azure-pipelines[bot] avatar Mar 13 '23 01:03 azure-pipelines[bot]

    @nikhilpadhye1, can you follow instructions to get breaking change approval?

    raych1 avatar Mar 13 '23 01:03 raych1

    This PR was signed off by arm team for 2022-09-01 but approvals didnt make the cut date. Closing this pr and making new one for 2022-11-01.

    nikhilpadhye1 avatar Mar 15 '23 19:03 nikhilpadhye1