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.

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

Adding AlertsSuppressionRule cmdlets to Az.Security #17763

Merged
merged 8 commits into from
Apr 26, 2022

Conversation

nitsi
Copy link
Contributor

@nitsi nitsi commented Apr 11, 2022

Description

Adding new capabilities for alerts suppression rule - user can create, delete, update and get rules (create and update are both the same "Set").

Checklist

  • I have read the Submitting Changes section of CONTRIBUTING.md
  • The title of the PR is clear and informative
  • The appropriate ChangeLog.md file(s) has been updated:
    • For any service, the ChangeLog.md file can be found at src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
    • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header -- no new version header should be added
  • The PR does not introduce breaking changes
  • If applicable, the changes made in the PR have proper test coverage
  • For public API changes to cmdlets:
    • a cmdlet design review was approved for the changes in this repository (Microsoft internal only)
    • the markdown help files have been regenerated using the commands listed here

Copy link
Member

@isra-fel isra-fel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please resolve the conflicts

src/Security/Security/Az.Security.psd1 Outdated Show resolved Hide resolved
@nitsi nitsi added needs-author-feedback More information is needed from author to address the issue. and removed needs-revision labels Apr 18, 2022
@nitsi
Copy link
Contributor Author

nitsi commented Apr 24, 2022

@isra-fel I'm getting the following error:
"Az.Security","Microsoft.Azure.Commands.Security.Cmdlets.AlertsSuppressionRules.NewAlertsSuppressionRuleScope","New-AzAlertsSuppressionRuleScope","1","8100","New-AzAlertsSuppressionRuleScope Does not support ShouldProcess but the cmdlet verb New indicates that it should.","Determine if the cmdlet should implement ShouldProcess and if so determine if it should implement Force / ShouldContinue"

NewAlertsSuppressionRuleScope has SupportsShouldProcess = false do I need to support ShouldProcess in this case, as it creates a local variable not a remote resource?

@isra-fel
Copy link
Member

@nitsi no, in-memory cmdlets do not support ShouldProcess.
CI failed simply because the rule was set to detect by cmdlet name (verb).
Please follow this guide to suppress the error: https://github.com/Azure/azure-powershell/blob/main/documentation/Debugging-StaticAnalysis-Errors.md#breaking-changes

@isra-fel isra-fel added needs-revision and removed needs-author-feedback More information is needed from author to address the issue. labels Apr 25, 2022
@isra-fel isra-fel removed their assignment Apr 25, 2022
@nitsi nitsi added needs-author-feedback More information is needed from author to address the issue. and removed needs-revision labels Apr 25, 2022
@nitsi
Copy link
Contributor Author

nitsi commented Apr 25, 2022

@isra-fel tests and static analysis passed, "azure-powershell - security-tools" have not started yet (2h), is there a way to restart it?

@nitsi nitsi assigned isra-fel and unassigned nitsi Apr 25, 2022
@isra-fel
Copy link
Member

/azp run azure-powershell - security-tools

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 1 pipeline(s).

@isra-fel isra-fel added Waiting for CI :shipit: and removed needs-author-feedback More information is needed from author to address the issue. labels Apr 25, 2022
@nitsi
Copy link
Contributor Author

nitsi commented Apr 25, 2022

@isra-fel all pipelines finished successfully over an hour ago, what is the next step?

@isra-fel isra-fel merged commit e2d5ebc into Azure:main Apr 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants