-
Notifications
You must be signed in to change notification settings - Fork 89
adds function Copy-NsxIpset #627
base: master
Are you sure you want to change the base?
adds function Copy-NsxIpset #627
Conversation
Hi j33tu Thanks for contribution but i think, it is more a tool, may be move on Example folder ? |
hi @alagoutte thanks for feedback , added to example folder , could you have a look and if looks good merge plz |
was some conflict which has been fix @alagoutte should be good to go now |
@alagoutte hope we are good to merge this one |
Thanks, Can you look https://github.com/vmware/powernsx/blob/master/tools/DiagramNSX/NsxObjectCapture.ps1 for remove function (no need) and also require PowerNSX and i don't have the magic button for merge @dcoghlan can you look ? |
@dcoghlan please help on this |
@@ -0,0 +1,57 @@ | |||
function Copy-NsxIpSet { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add any version or module requirements at the top here. Maybe take a look at the following as an example
https://github.com/vmware/powernsx/blob/master/Examples/EnableFirewallRuleLogging.ps1
Also please add Author details, contact information and script/function/cmdlet versioning
[parameter(Mandatory = $true)] | ||
[string] $SecondaryNsxManager, | ||
[parameter (Mandatory = $true)] | ||
[pscredential] $credential |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how does it handle different credentials for the primary and secondary managers?
begin | ||
{ | ||
Connect-NsxServer $PrimaryNsxManager -DisableVIAutoConnect -Credential $credential | ||
$NsxIpSets = @(Get-NsxIpSet) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if the primary one has both global and universal ipsets? is the intention that all will be copied?
} | ||
else{ | ||
Write-Verbose -Message "Not Found ip set, Creating NsxIpSet $($NsxIpSet.Name)" | ||
New-NsxIpSet -Name $NsxIpSet.Name -IPAddress $NsxIpSet.value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scope should be specified, so that ipsets are recreated appropraitely
} | ||
else{ | ||
Write-Verbose -Message "Not Found ip set, Creating NsxIpSet $($NsxIpSet.Name)" | ||
New-NsxIpSet -Name $NsxIpSet.Name -IPAddress $NsxIpSet.value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about inheritance?
adding a new Function Copy-Nsx-IpSet