Skip to content

Introducing Az.Security AllowedConnection cmdlets#12233

Merged
VeryEarly merged 24 commits intoAzure:masterfrom
ariklin:master
Jul 3, 2020
Merged

Introducing Az.Security AllowedConnection cmdlets#12233
VeryEarly merged 24 commits intoAzure:masterfrom
ariklin:master

Conversation

@ariklin
Copy link
Copy Markdown
Contributor

@ariklin ariklin commented Jun 22, 2020 •
edited
Loading

Description

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

Design review: https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/619

@adxsdkps
Copy link
Copy Markdown
Collaborator

adxsdkps commented Jun 22, 2020

Can one of the admins verify this patch?

@ariklin ariklin marked this pull request as ready for review June 22, 2020 14:55
@ariklin ariklin requested a review from VeryEarly June 22, 2020 14:55
@VeryEarly VeryEarly self-assigned this Jun 23, 2020
VeryEarly
VeryEarly reviewed Jun 23, 2020
View reviewed changes
VeryEarly
VeryEarly reviewed Jun 23, 2020
View reviewed changes
VeryEarly
VeryEarly reviewed Jun 23, 2020
View reviewed changes
VeryEarly
VeryEarly reviewed Jun 23, 2020
View reviewed changes
@ariklin ariklin changed the title Introducing Az.Security AllowedConnections cmdlets Introducing Az.Security AllowedConnection cmdlets Jun 23, 2020
@ariklin
Copy link
Copy Markdown
Contributor Author

ariklin commented Jun 26, 2020

@VeryEarly, please consider my last comment.

@ariklin
Copy link
Copy Markdown
Contributor Author

ariklin commented Jun 28, 2020

@VeryEarly, please consider my last commit.

VeryEarly
VeryEarly requested changes Jun 29, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@VeryEarly VeryEarly left a comment •
edited
Loading

Choose a reason for hiding this comment

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

  • there should be another parameter set where only "-Location" is mandatory, to support list by home region

  • regenerate help with these changes

  • please add test case to cover these scenarios

@ariklin
Copy link
Copy Markdown
Contributor Author

ariklin commented Jun 29, 2020

@VeryEarly, List by home region only is not supported by manual api.

@VeryEarly
Copy link
Copy Markdown
Collaborator

VeryEarly commented Jun 29, 2020

@VeryEarly, List by home region only is not supported by manual api.

do you mean https://docs.microsoft.com/en-us/rest/api/securitycenter/allowedconnections/listbyhomeregion this is not working?

VeryEarly
VeryEarly reviewed Jun 29, 2020
View reviewed changes
VeryEarly
VeryEarly reviewed Jun 29, 2020
View reviewed changes
@ariklin
Copy link
Copy Markdown
Contributor Author

ariklin commented Jun 29, 2020

  • there should be another parameter set where only "-Location" is mandatory, to support list by home region
  • regenerate help with these changes
  • please add test case to cover these scenarios

@VeryEarly, List by home region only is not supported by manual api.

do you mean https://docs.microsoft.com/en-us/rest/api/securitycenter/allowedconnections/listbyhomeregion this is not working?

it doesn't

@VeryEarly
Copy link
Copy Markdown
Collaborator

VeryEarly commented Jun 30, 2020

  • there should be another parameter set where only "-Location" is mandatory, to support list by home region
  • regenerate help with these changes
  • please add test case to cover these scenarios

@VeryEarly, List by home region only is not supported by manual api.

do you mean https://docs.microsoft.com/en-us/rest/api/securitycenter/allowedconnections/listbyhomeregion this is not working?

it doesn't

I tried call GET /subscriptions/{subscription}/providers/Microsoft.Security/locations/westcentralus/allowedConnections?api-version=2020-01-01

had successful response, how is it not supported?

@ariklin
Copy link
Copy Markdown
Contributor Author

ariklin commented Jul 1, 2020

  • there should be another parameter set where only "-Location" is mandatory, to support list by home region
  • regenerate help with these changes
  • please add test case to cover these scenarios

@VeryEarly, List by home region only is not supported by manual api.

do you mean https://docs.microsoft.com/en-us/rest/api/securitycenter/allowedconnections/listbyhomeregion this is not working?

it doesn't

I tried call GET /subscriptions/{subscription}/providers/Microsoft.Security/locations/westcentralus/allowedConnections?api-version=2020-01-01

had successful response, how is it not supported?

@VeryEarly, as a PS command with listbyhomeregion will be useless, as a subscription can have only one ASC Location.
In PS, in a context of a subscription, the ASC location will have only one valid value.

A user may determine the ASC Location of his subscription using the following:

GET https://management.azure.com/subscriptions/{subscriptionId}/providers/Microsoft.Security/locations/{ascLocation}?api-version=2015-06-01-preview

@VeryEarly
Copy link
Copy Markdown
Collaborator

VeryEarly commented Jul 1, 2020

  • there should be another parameter set where only "-Location" is mandatory, to support list by home region
  • regenerate help with these changes
  • please add test case to cover these scenarios

@VeryEarly, List by home region only is not supported by manual api.

do you mean https://docs.microsoft.com/en-us/rest/api/securitycenter/allowedconnections/listbyhomeregion this is not working?

it doesn't

I tried call GET /subscriptions/{subscription}/providers/Microsoft.Security/locations/westcentralus/allowedConnections?api-version=2020-01-01
had successful response, how is it not supported?

@VeryEarly, as a PS command with listbyhomeregion will be useless, as a subscription can have only one ASC Location.
In PS, in a context of a subscription, the ASC location will have only one valid value.

A user may determine the ASC Location of his subscription using the following:

GET https://management.azure.com/subscriptions/{subscriptionId}/providers/Microsoft.Security/locations/{ascLocation}?api-version=2015-06-01-preview

Do you mean list by subscription and list by home region will have the same response?

@ariklin
Copy link
Copy Markdown
Contributor Author

ariklin commented Jul 1, 2020

  • there should be another parameter set where only "-Location" is mandatory, to support list by home region
  • regenerate help with these changes
  • please add test case to cover these scenarios

@VeryEarly, List by home region only is not supported by manual api.

do you mean https://docs.microsoft.com/en-us/rest/api/securitycenter/allowedconnections/listbyhomeregion this is not working?

it doesn't

I tried call GET /subscriptions/{subscription}/providers/Microsoft.Security/locations/westcentralus/allowedConnections?api-version=2020-01-01
had successful response, how is it not supported?

@VeryEarly, as a PS command with listbyhomeregion will be useless, as a subscription can have only one ASC Location.
In PS, in a context of a subscription, the ASC location will have only one valid value.
A user may determine the ASC Location of his subscription using the following:
GET https://management.azure.com/subscriptions/{subscriptionId}/providers/Microsoft.Security/locations/{ascLocation}?api-version=2015-06-01-preview

Do you mean list by subscription and list by home region will have the same response?

Yes, it will.

@VeryEarly
Copy link
Copy Markdown
Collaborator

VeryEarly commented Jul 1, 2020

  • there should be another parameter set where only "-Location" is mandatory, to support list by home region
  • regenerate help with these changes
  • please add test case to cover these scenarios

@VeryEarly, List by home region only is not supported by manual api.

do you mean https://docs.microsoft.com/en-us/rest/api/securitycenter/allowedconnections/listbyhomeregion this is not working?

it doesn't

I tried call GET /subscriptions/{subscription}/providers/Microsoft.Security/locations/westcentralus/allowedConnections?api-version=2020-01-01
had successful response, how is it not supported?

@VeryEarly, as a PS command with listbyhomeregion will be useless, as a subscription can have only one ASC Location.
In PS, in a context of a subscription, the ASC location will have only one valid value.
A user may determine the ASC Location of his subscription using the following:
GET https://management.azure.com/subscriptions/{subscriptionId}/providers/Microsoft.Security/locations/{ascLocation}?api-version=2015-06-01-preview

Do you mean list by subscription and list by home region will have the same response?

Yes, it will.

If that's the case, why "-Location" parameter is needed?

@ariklin
Copy link
Copy Markdown
Contributor Author

ariklin commented Jul 1, 2020

  • there should be another parameter set where only "-Location" is mandatory, to support list by home region
  • regenerate help with these changes
  • please add test case to cover these scenarios

@VeryEarly, List by home region only is not supported by manual api.

do you mean https://docs.microsoft.com/en-us/rest/api/securitycenter/allowedconnections/listbyhomeregion this is not working?

it doesn't

I tried call GET /subscriptions/{subscription}/providers/Microsoft.Security/locations/westcentralus/allowedConnections?api-version=2020-01-01
had successful response, how is it not supported?

@VeryEarly, as a PS command with listbyhomeregion will be useless, as a subscription can have only one ASC Location.
In PS, in a context of a subscription, the ASC location will have only one valid value.
A user may determine the ASC Location of his subscription using the following:
GET https://management.azure.com/subscriptions/{subscriptionId}/providers/Microsoft.Security/locations/{ascLocation}?api-version=2015-06-01-preview

Do you mean list by subscription and list by home region will have the same response?

Yes, it will.

If that's the case, why "-Location" parameter is needed?
It is required by ARM.

VeryEarly
VeryEarly requested changes Jul 2, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@VeryEarly VeryEarly left a comment

Choose a reason for hiding this comment

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

please change this back to 0.7.9
https://github.com/ariklin/azure-powershell-1/blob/master/src/Security/Security/Az.Security.psd1#L15

@ariklin
Copy link
Copy Markdown
Contributor Author

ariklin commented Jul 2, 2020

please change this back to 0.7.9
https://github.com/ariklin/azure-powershell-1/blob/master/src/Security/Security/Az.Security.psd1#L15

Done.

@VeryEarly VeryEarly merged commit 21719e6 into Azure:master Jul 3, 2020
litchiyangMSFT pushed a commit to litchiyangMSFT/azure-powershell that referenced this pull request Aug 4, 2020
@ariklin @VeryEarly @litchiyangMSFT
Introducing Az.Security AllowedConnection cmdlets (Azure#12233)
b791798 
* Introducing SecurityTopologies cmdlets

* Update ChangeLog.md

* Update ChangeLog.md

* Change cmdlet name to Get-AzSecurityTopology

* remove AllowedConnections

* change name

* fix conflicy

* fix conflict

* Update ChangeLog.md

* Introducing Az.Security AllowedConnections cmdlets

* update change.log

* remove SecurityTopology

* fix version

* Fix comments

* fix location

* fix comment

* fix comment

* fix version number

Co-authored-by: Arik Riklin <arik.riklin@microsoft.com>
Co-authored-by: Yabo Hu <yabhu@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@VeryEarly VeryEarly VeryEarly approved these changes

Assignees

@VeryEarly VeryEarly

Labels

needs-review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ariklin @adxsdkps @VeryEarly @azuresdkci