Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Hub Generated] Review request for Microsoft.Security to add version stable/2019-08-01 #8820

Merged
merged 2 commits into from
Apr 8, 2020
Merged

[Hub Generated] Review request for Microsoft.Security to add version stable/2019-08-01 #8820

merged 2 commits into from
Apr 8, 2020

Conversation

liranc
Copy link
Contributor

@liranc liranc commented Mar 24, 2020 •
edited
Loading

If you are a MSFT employee you can view your work branch via this link.

Contribution checklist:

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liranc
Copy link
Contributor Author

liranc commented Mar 24, 2020

@xccc-msft I believe the public.rest-api-specs (BreakingChange) gate failure is a false positive.
The latest version of the swagger results in an error when attempt to generate code using AutoRest.
The issue is that the ruleType field was defined both in CustomAlertRule (the base type) and AllowlistCustomAlertRule (deriving type). Also, it wasn't marked as a discriminator even though it behaved like one.
So the changes in the PR remove the duplicate definition of the property and declare it as a discriminator.
As far as I understand this change has no affect on the API itself, so there should be no reason to update the api-version.
Is that correct?

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Mar 24, 2020 •
edited
Loading

azure-cli-extensions - Release

No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Mar 24, 2020 •
edited
Loading

azure-sdk-for-java - Release

No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Mar 24, 2020 •
edited
Loading

azure-sdk-for-js - Release

️✔️ succeeded [Logs] [Expand Details]

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Mar 24, 2020 •
edited
Loading

azure-sdk-for-python - Release

- Breaking Change detected in SDK

⚠️ warning [Logs] [Expand Details]
  • ⚠️ Generate from 6fbbb21 with merge commit 51f9f41. SDK Automation 13.0.17.20200326.3
  • ⚠️azure-mgmt-security [Logs]  [Release SDK Changes] Breaking Change Detected
    [build_package] /usr/lib/python3.6/distutils/dist.py:261: UserWarning: Unknown distribution option: 'long_description_content_type'
[build_package]   warnings.warn(msg)
[build_package] warning: no files found matching '*.py' under directory 'tests'
[build_package] warning: no files found matching '*.yaml' under directory 'tests'
[build_package] /usr/lib/python3.6/distutils/dist.py:261: UserWarning: Unknown distribution option: 'long_description_content_type'
[build_package]   warnings.warn(msg)
[build_package] warning: no files found matching '*.py' under directory 'tests'
[build_package] warning: no files found matching '*.yaml' under directory 'tests'
[breaking_change_setup] Ignoring mock: markers 'python_version <= "2.7"' don't match your environment
[breaking_change_setup] Cannot uninstall requirement azure-nspkg, not installed
[breaking_change_setup] Command '['/usr/local/bin/python', '-m', 'pip', 'uninstall', '-y', 'azure-nspkg']' returned non-zero exit status 1.
[ChangeLog] Size of delta 45.774% size of original (original: 131348 chars, delta: 60123 chars)
[ChangeLog] **Features**
[ChangeLog] 
[ChangeLog]   - Model AppWhitelistingGroup has a new parameter protection_mode
[ChangeLog]   - Model JitNetworkAccessPolicyInitiateRequest has a new parameter justification
[ChangeLog]   - Model JitNetworkAccessRequest has a new parameter justification
[ChangeLog]   - Model VmRecommendation has a new parameter enforcement_support
[ChangeLog]   - Model InformationType has a new parameter description
[ChangeLog]   - Model AppWhitelistingPutGroupData has a new parameter protection_mode
[ChangeLog]   - Model SensitivityLabel has a new parameter description
[ChangeLog]   - Model SensitivityLabel has a new parameter rank
[ChangeLog]   - Model IoTSecurityAggregatedAlert has a new parameter top_devices_list
[ChangeLog]   - Model IoTSecuritySolutionModel has a new parameter unmasked_ip_logging_status
[ChangeLog]   - Added operation AdaptiveApplicationControlsOperations.delete
[ChangeLog]   - Added operation IotSecuritySolutionOperations.list_by_subscription
[ChangeLog]   - Added operation IotSecuritySolutionOperations.list_by_resource_group
[ChangeLog]   - Added operation IotSecuritySolutionOperations.create_or_update
[ChangeLog]   - Added operation AlertsOperations.update_subscription_level_alert_state_to_reactivate
[ChangeLog]   - Added operation AlertsOperations.update_subscription_level_alert_state_to_dismiss
[ChangeLog]   - Added operation AlertsOperations.update_resource_group_level_alert_state_to_reactivate
[ChangeLog]   - Added operation AlertsOperations.update_resource_group_level_alert_state_to_dismiss
[ChangeLog]   - Added operation group IotSecuritySolutionsAnalyticsAggregatedAlertOperations
[ChangeLog]   - Added operation group SubAssessmentsOperations
[ChangeLog]   - Added operation group IotSecuritySolutionsAnalyticsRecommendationOperations
[ChangeLog]   - Added operation group AutomationsOperations
[ChangeLog]   - Added operation group AssessmentsOperations
[ChangeLog]   - Added operation group IotSecuritySolutionAnalyticsOperations
[ChangeLog]   - Added operation group AssessmentsMetadataOperations
[ChangeLog]   - Added operation group AdaptiveNetworkHardeningsOperations
[ChangeLog]   - Added operation group DeviceSecurityGroupsOperations
[ChangeLog] 
[ChangeLog] **Breaking changes**
[ChangeLog] 
[ChangeLog]   - Operation JitNetworkAccessPoliciesOperations.initiate has a new signature
[ChangeLog]   - Operation InformationProtectionPoliciesOperations.create_or_update has a new signature
[ChangeLog]   - Removed operation IotSecuritySolutionOperations.create
[ChangeLog]   - Removed operation AlertsOperations.update_subscription_level_alert_state
[ChangeLog]   - Removed operation AlertsOperations.update_resource_group_level_alert_state
[ChangeLog]   - Removed operation group IoTSecuritySolutionsAnalyticsAggregatedAlertOperations
[ChangeLog]   - Removed operation group IoTSecuritySolutionsAnalyticsOperations
[ChangeLog]   - Removed operation group IoTSecuritySolutionsOperations
[ChangeLog]   - Removed operation group IoTSecuritySolutionsAnalyticsAggregatedAlertsOperations
[ChangeLog]   - Removed operation group IoTSecuritySolutionsAnalyticsRecommendationsOperations
[ChangeLog]   - Removed operation group IoTSecuritySolutionsResourceGroupOperations
[ChangeLog]   - Removed operation group IoTSecuritySolutionsAnalyticsRecommendationOperations

@openapi-sdkautomation openapi-sdkautomation bot mentioned this pull request Mar 24, 2020
Closed
@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Mar 24, 2020 •
edited
Loading

azure-sdk-for-net - Release

️✔️ succeeded [Logs] [Expand Details]

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Mar 24, 2020 •
edited
Loading

azure-sdk-for-go - Release

️✔️ succeeded [Logs] [Expand Details]

This was referenced Mar 24, 2020
Closed
Closed
@xseeseesee
Copy link
Contributor

@NullMDR Can you please advise for the CIs?

@xseeseesee xseeseesee added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Mar 25, 2020
@PhoenixHe-NV
Copy link
Contributor

@xccc-msft I believe the public.rest-api-specs (BreakingChange) gate failure is a false positive.
The latest version of the swagger results in an error when attempt to generate code using AutoRest.
The issue is that the ruleType field was defined both in CustomAlertRule (the base type) and AllowlistCustomAlertRule (deriving type). Also, it wasn't marked as a discriminator even though it behaved like one.
So the changes in the PR remove the duplicate definition of the property and declare it as a discriminator.
As far as I understand this change has no affect on the API itself, so there should be no reason to update the api-version.
Is that correct?

@jianyexi Could you please help on the breaking change detection in CI?

@liranc
Copy link
Contributor Author

liranc commented Apr 1, 2020 •
edited
Loading

@xccc-msft @NullMDR @jianyexi Hi, any updates on this PR?

@jianyexi
Copy link
Contributor

jianyexi commented Apr 1, 2020 •
edited
Loading

@xccc-msft @NullMDR @jianyexi Hi, any updates on this PR?
seems make sense , I think it's a false positive
currently we can go on reviewing , and ask @akning-ms to merge it at last

@akning-ms
Copy link
Contributor

Hi @liranc, Seems you are just fix duplicate RuleType issue in this PR. So removed ARM review request. agree it is not breaking change for api. but I am not sure for sdk. can you share me about what language sdk to be released for this PR? I may ask sdk owner to take a look.

@akning-ms akning-ms removed the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Apr 2, 2020
@liranc
Copy link
Contributor Author

liranc commented Apr 5, 2020

Hi @liranc, Seems you are just fix duplicate RuleType issue in this PR. So removed ARM review request. agree it is not breaking change for api. but I am not sure for sdk. can you share me about what language sdk to be released for this PR? I may ask sdk owner to take a look.

@akning-ms The released SDK is C#. The generated code will look a bit different and possibly break existing code, but is this an issue since the NuGet is always published using a different version? (so only those who would upgrade to the new version will need to make these few adjustments).
Thanks!

@akning-ms
Copy link
Contributor

approved, but @isra-fel for awareness .Net sdk

@akning-ms akning-ms merged commit 51f9f41 into Azure:master Apr 8, 2020
@liranc liranc deleted the dev-security-Microsoft.Security-2019-08-01 branch May 18, 2020 08:09
@liranc liranc mentioned this pull request Aug 20, 2020
Merged
10 tasks
@liranc liranc mentioned this pull request Sep 14, 2020
Merged
10 tasks
00Kai0 pushed a commit to 00Kai0/azure-rest-api-specs that referenced this pull request Oct 12, 2020
@liranc @00Kai0
[Hub Generated] Review request for Microsoft.Security to add version …
a2645b9 
…stable/2019-08-01 (Azure#8820)

* Set ruleType as discriminator. Create concrete rule types

* ran prettier

Co-authored-by: Liran Chen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mikreite mikreite Awaiting requested review from mikreite

@chlahav chlahav Awaiting requested review from chlahav

@dapickMS dapickMS Awaiting requested review from dapickMS

Assignees

@xseeseesee xseeseesee

Labels
potential-sdk-breaking-change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@liranc @azuresdkci @xseeseesee @PhoenixHe-NV @jianyexi @akning-ms