optimize Get-NsxSecurityTagAssignment per #541

[mtboren]

Updated the VirtualMachine parameterset code for getting assigned NSX Security Tags by VM:

• uses GET method /api/2.0/services/securitytags/vm/{vmId} that came with NSX v6.3.0
• still returns one object per assigned Security Tag (or $null if no tags assigned to given VM)
• nice performance bump in test environment of 8,000 VMs and ~40 security tags; for getting assigned tag for one VM:
  • took ~7 minutes (424.6290539 seconds) with know-to-be-less-than-optimal method
  • took ~1 second with updated method

[vmwclabot]

@mtboren, you must sign our contributor license agreement before your changes are merged. Click here to sign the agreement. If you are a VMware employee, read this for further instruction.

[vmwclabot]

@mtboren, VMware has approved your signed contributor license agreement.

[nmbradford]

LGTM! Thanks for a quality PR. Any change you could write some simple pester tests for it so we can merge asap?

[mtboren]

Super. Welcome, glad to help make things even better!

Yes, there is about a 100% chance. I have now done so, and have done a git commit --amend as described in CONTRIBUTING.md.

Let me know what else we need in order to wrap up this addition.

And, an aside: in creating those tests, I saw a couple of other opportunities for iterating on the module and continuing to make it better still -- the credentials gathering in the Start-Test function, and doing so a bit more securely (in a way that does not put passwords into PowerShell history). I'll open an issue to track the idea.

[nmbradford]

Great work - thanks @mtboren ! I notice the tests expect security tags to pre-exist, can you add creation (and cleanup) to the before all/after all. Tests need to be able to run on a (mostly) vanilla deployment.

[mtboren]

Thanks.

I certainly can add such things. Though, does that mean, too, that the tests will also need to take care of creating and cleaning up a subject VM and a target security tag assignment, if the tests are to assume that there may be zero tag assignments in the vanilla deployment, as well?

[nmbradford]

Correct. There are examples in some of the other tests like the DFW ones that you could copy.

[mtboren]

Got it, and, done. The PR's "Files changed" view reflects the overall things that I have added for both the function update and the tests. Let me know if there are any other things to address before a merge.

Aside: I am not too familiar w/ the effects of the --amend argument on the git commit, but see that the commit/push that I just made shows as "4 days ago" in the related text above in the comment thread. Not sure that makes much matter, just observing.

Cheers

[nmbradford]

Thanks @mtboren - I for one certainly have no issue with you making code more robust than our existing!!! At first glance the tests you've added are precisely whats required. I will take a more detailed look later today when I get a free moment.

[nmbradford]

jenkins test this please

[powernsxbot]

Tests Failed

[nmbradford]

@mtboren - great work - your tests pass (we have a recurring issue with a few of our security policy tests that caused the failure that are unrelated to your changes). Thanks for a quality PR!

[mtboren]

Greets, @nmbradford -- thanks much! I am enjoying the module, and am glad to have to opportunity to contribute. And, thanks for the additional info. See you on the next enhancement