Skip to content
This repository has been archived by the owner on Oct 28, 2022. It is now read-only.

adds function Copy-NsxIpset #627

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

j33tu
Copy link

@j33tu j33tu commented Jul 8, 2020

adding a new Function Copy-Nsx-IpSet

@alagoutte
Copy link
Contributor

Hi j33tu

Thanks for contribution but i think, it is more a tool, may be move on Example folder ?

@j33tu
Copy link
Author

j33tu commented Jul 13, 2020

hi @alagoutte thanks for feedback , added to example folder , could you have a look and if looks good merge plz

@j33tu
Copy link
Author

j33tu commented Jul 13, 2020

was some conflict which has been fix @alagoutte should be good to go now

@j33tu
Copy link
Author

j33tu commented Jul 16, 2020

@alagoutte hope we are good to merge this one

@alagoutte
Copy link
Contributor

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 ?

@j33tu
Copy link
Author

j33tu commented Jul 17, 2020

@dcoghlan please help on this

@@ -0,0 +1,57 @@
function Copy-NsxIpSet {
Copy link
Contributor

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
Copy link
Contributor

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)
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

Choose a reason for hiding this comment

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

What about inheritance?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants