Improve Get-NsxSecurityTagAssignment performance for VirtualMachine parameterset

[mtboren]

Greets-

Still enjoying the module -- thanks again!

The Get-NsxSecurityTagAssignment cmdlet has a known opportunity for enhancement in the VirtualMachine parameterset (the parameter set in which a VM/template is provided for the value of the -VirtualMachine parameter): getting the Security Tags for exactly that VM/template, rather than getting all Security Tags and seeing if each/any has an assignment to the given VM/template (this performance situation is already mentioned in issue #333).

I see the 'Ill be back...' comment in the code, and understand that this is a, "get to it someday". I'm opening this GH Issue to track this, and to include info about the API call that will likely get us to the high performance that we want for this cmdlet/parameterset.

• The NSX API Guide section: Working With Security Tags on a Specific Virtual Machine
• The API Get URI that looks like a winner: /api/2.0/services/securitytags/vm/{vmId}
• The update to the VirtualMachine case in the switch statement in the Get-NsxSecurityTagAssignment function definition will likely include:
  • using this aforementioned URI in place of the current Get-NsxSecurityTag -connection $connection | Get-NsxSecurityTagAssignment ... bits of code
  • iterating over the $VirtualMachine parameter value, since that accepts one or more VM/template object

I may whip up some code for consideration as a solution -- I will comment back here if so, before submitting a PR.

Cheers, Matt

[mtboren]

Oh, and, I see that this API method was introduced in NSX 6.3.0, so we would likely need to keep some "get tag assignment by the current means if [System.Version]$connection.Version -lt [System.Version]"6.3.0"" kind of logic in that VirtualMachine case.

[nmbradford]

Thanks for the comment Matt - a PR would be most welcome. Support for pre 6.3.0 deployments will be moot in a week and a half when 6.2.x goes end of support on Aug 20 (https://kb.vmware.com/s/article/52440), so I'm not too concerned about leveraging APIs that were introduced in 6.3 but if you want to include it that's fine too.

[mtboren]

Ok, got it, @nmbradford -- I'll exclude support for unsupported-within-a-week versions less than 6.3.0. And, I've now pushed commit e1dd3d7 to my fork that adds the optimization as I described above. I'll submit a PR shortly.

Let me know if there are any things in the commit that we should discuss.

[nmbradford]

resolved in #545

[mtboren]

Aw, snap -- I might have found one further thing that I need to add to complete this update: the return from Get-VM someVmWithTwoTagAssignments | Get-NsxSecurityTagAssignment should be two objects, each with properties SecurityTag and VirtualMachine, right? I just confirmed that was the behavior before the update in #545. However, the behavior now seems to be a single object per VM instead of per security tag assigned, with a SecurityTag property that is an array of SecurityTag XML objects.

To illustrate:

## return before #545, which is desired, I think:
PS C:\> Get-VM someVmWithTwoTagAssignments | Get-NsxSecurityTagAssignment
SecurityTag    VirtualMachine
-----------    --------------
securityTag    someVmWithTwoTagAssignments
securityTag    someVmWithTwoTagAssignments


## behavior after #545 (not in keeping with previous behavior):
PS C:\> Get-VM someVmWithTwoTagAssignments | Get-NsxSecurityTagAssignment
SecurityTag         VirtualMachine
-----------         --------------
{MyTag0, MyTag1}    someVmWithTwoTagAssignments

Assuming that we want to continue the behavior that was in place before update #545, I'll make that correction straight away, update the test code to check for the desired behavior, test, and submit a PR for review.

Do we indeed want to continue the previous behavior?

[nmbradford]

Bummer, good catch. Yeah - we try hard to avoid breaking changes, so if you have it in you to revert to previous behaviour that would be great. Thanks for the update.

[mtboren]

Greets, @nmbradford -

You may have already seen the commit and then PR (#550) from a few days ago (which itself creates a notification, I think), but I'm making this comment just to get [another?] notification to you. Review at your leisure, of course. Have a good weekend

[nmbradford]

Thanks @mtboren - yup - got the notification, just dealing with lead up to VMworld :)