toolbox icon indicating copy to clipboard operation
toolbox copied to clipboard

vCenter Example

Open dafyre opened this issue 3 years ago • 2 comments

Allows for pulling VMs into folders with the same layout that vCenter has.

dafyre avatar Apr 18 '22 14:04 dafyre

Thx for the PR @dafyre! Any chance you could test this on macOS as well?

lemonmojo avatar Apr 18 '22 15:04 lemonmojo

@dafyre I just tested the script on macOS using PowerShell 7.2.1 and it worked fine. By the way, I ran it directly against an ESXi so just FYI it seems to work fine without a vCenter as a target.

I'd still like to see a couple of improvements before I'll merge it into our repository:

  1. Use single quoted strings wherever you use "$-tokens". So $vcServers = "$CustomProperty.ServerURL$" would become $vcServers = '$CustomProperty.ServerURL$' and the $EffectiveUsername$ and $EffectivePassword$ aren't quoted at all right now. They should also use single quotes.
  2. Please add some more info to the notes section. For instance, all scripts in this repo should have the author's name (your Github handle is fine if you don't want your full name in there) in the notes, the version of the script and sections like an introduction, requirements, and setup steps if required. Here's an example (from the 1Password script):

1Password Dynamic Folder sample

Version: 2.0.1 Author: Royal Apps

This Dynamic Folder sample allows you to import credentials from 1Password. The 1Password command-line tool is required and the path where it is installed must be configured in the "Custom Properties" section. Also, your 1Password login details must be provided in the "Login" section of the custom properties.

At the moment, items of all vaults are imported as Dynamic Credentials. This means that the username and password fields will remain empty after reloading the dynamic folder and only be requested when a connection is established that uses one of the credentials of this dynamic folder.

Requirements

1Password command-line tool (Version 1 and 2 are supported) Biometric unlock for 1Password CLI must be disabled in the 1Password app Python 3 (Python 2 is not supported) Python Module: pexpect on macOS, wexpect on Windows Python Module: future Python Module: sys Python Module: functools Python Module: json Python Module: subprocess Python Module: sys Python Module: os Python Module: base64 Python Module: tempfile

Setup

Specify the full, absolute path to the 1Password command-line tool in the "Custom Properties" section. Specify your login details for accessing your 1Password account in the "Login" section of the "Custom Properties".

lemonmojo avatar May 05 '22 13:05 lemonmojo

@dafyre Any chance you could integrate the mentioned changes into your script? We'd love to merge it.

lemonmojo avatar Feb 10 '23 15:02 lemonmojo

@dafyre Any chance you could integrate the mentioned changes into your script? We'd love to merge it.

@lemonmojo -- I seem to have missed a lot of emails from Github lately. I will check into this and get it updated on Monday assuming nothing goes nuts over the weekend.

Also thanks for testing it on MacOS -- I don't have a working Mac at the moment. Looking to fix that at my day job, lol.

dafyre avatar Feb 10 '23 22:02 dafyre

@dafyre Awesome, thx!

lemonmojo avatar Feb 11 '23 06:02 lemonmojo

@dafyre Your previous comment seems to have vanished but since I still have the email notification I'll answer it nonetheless.

The phenomenon you're describing is exactly what I'm trying to prevent with the suggestion to move strings that contain Royal $ tokens to single quoted strings.

The problem is this: PowerShell uses $ to do string interpolation. If you use $yourVariable = "$RoyalProperty$" and RoyalProperty actually contains a $ that would cause PowerShell to interpret it as a variable and do string interpolation.

Here's an example: If you'd use $myPassword = "$Password$" and the password stored in the Dynamic Folder is Blah$Something, PowerShell would replace $Something with the variable $Something which probably doesn't exist.

So I'm not suggesting to use single quotes everywhere in your script but in all the cases where a Royal $ token is used.

Hope that clears up the confusion!

lemonmojo avatar Feb 13 '23 13:02 lemonmojo

Updates checked and tested working for me on Windows, PowerShell 5.1

dafyre avatar Feb 13 '23 14:02 dafyre

@dafyre Your previous comment seems to have vanished but since I still have the email notification I'll answer it nonetheless.

The phenomenon you're describing is exactly what I'm trying to prevent with the suggestion to move strings that contain Royal $ tokens to single quoted strings.

The problem is this: PowerShell uses $ to do string interpolation. If you use $yourVariable = "$RoyalProperty$" and RoyalProperty actually contains a $ that would cause PowerShell to interpret it as a variable and do string interpolation.

Here's an example: If you'd use $myPassword = "$Password$" and the password stored in the Dynamic Folder is Blah$Something, PowerShell would replace $Something with the variable $Something which probably doesn't exist.

So I'm not suggesting to use single quotes everywhere in your script but in all the cases where a Royal $ token is used.

Hope that clears up the confusion!

It does, I figured that out after thinking about it, hence the disappearing comment.

I just made the commit a little bit ago. I only found one place where that would have potentially been a problem. It has been fixed. Let me know what you think.

dafyre avatar Feb 13 '23 15:02 dafyre

@dafyre Thx! I'll take a look tomorrow.

lemonmojo avatar Feb 13 '23 15:02 lemonmojo

@dafyre Many thx for the changes. There are still two things that I'd like to see adjusted:

  1. Please add a (short) description (on the main page of the Dynamic Folder). This is used to show a quick summary of what the Dynamic Folder is capable of doing and is shown when importing dynamic folders through our in-app import dialog.
  2. In the notes, please to the required PS module as VMware.PowerCLI instead of VMware PowerCLI so that users can easily copy and paste the module for installation.

Cheers, Felix

lemonmojo avatar Feb 14 '23 10:02 lemonmojo

@lemonmojo Pushed the Description & the Notes update.

Seeing as this is actually more of a Generic VMware thing and not just vCenter, do we want to rename the main folder to VMware or leave it as vCenter?

Just a thought.

Thanks! ~Brant

On Tue, Feb 14, 2023 at 5:42 AM Felix Deimel @.***> wrote:

@dafyre https://github.com/dafyre Many thx for the changes. There are still two things that I'd like to see adjusted:

  1. Please add a (short) description (on the main page of the Dynamic Folder). This is used to show a quick summary of what the Dynamic Folder is capable of doing and is shown when importing dynamic folders through our in-app import dialog.
  2. In the notes, please to the required PS module as VMware.PowerCLI instead of VMware PowerCLI so that users can easily copy and paste the module for installation.

Cheers, Felix

— Reply to this email directly, view it on GitHub https://github.com/royalapplications/toolbox/pull/38#issuecomment-1429508112, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADF74TQIMHEEJJSJU4Z7IVDWXNOQRANCNFSM5TWBRYRQ . You are receiving this because you were mentioned.Message ID: @.***>

dafyre avatar Feb 14 '23 13:02 dafyre

@dafyre Yes, that makes sense.

I'd suggest the following additional changes:

  1. Replace all occurrences (in Folder name, File name, description, notes, etc.) of "vCenter" with either "VMware" or "VMware VMs" depending on the context.
  2. Replace the pre-filled "TODO" values for "WindowsCredential" and "LinuxCredential" in the export file with just a blank string. This ensures that users that import your dynamic folder don't get prompted for a credential named "TODO" if they don't configure these values. Note that you will have to do this in a text editor after exporting the dynamic folder with Royal TS/X.
  3. In the notes section, add a short reference to those properties so that it's clear to users that they are supposed to optionally enter a credential name(!) there but can leave them blank as well.

thx, Felix

lemonmojo avatar Feb 15 '23 07:02 lemonmojo

@lemonmojo

Thanks for the suggestions. They've been done and tweaked the notes a bit where it made sense. I did remove "TODO" from the credentials and updated the notes for that as well.

Thanks! ~Brant

On Wed, Feb 15, 2023 at 2:54 AM Felix Deimel @.***> wrote:

@dafyre https://github.com/dafyre Yes, that makes sense.

I'd suggest the following additional changes:

  1. Replace all occurrences (in Folder name, File name, description, notes, etc.) of "vCenter" with either "VMware" or "VMware VMs" depending on the context.
  2. Replace the pre-filled "TODO" values for "WindowsCredential" and "LinuxCredential" in the export file with just a blank string. This ensures that users that import your dynamic folder don't get prompted for a credential named "TODO" if they don't configure these values. Note that you will have to do this in a text editor after exporting the dynamic folder with Royal TS/X.
  3. In the notes section, add a short reference to those properties so that it's clear to users that they are supposed to optionally enter a credential name(!) there but can leave them blank as well.

thx, Felix

— Reply to this email directly, view it on GitHub https://github.com/royalapplications/toolbox/pull/38#issuecomment-1430896511, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADF74TSIJHEKDPXCVQJ7NV3WXSDULANCNFSM5TWBRYRQ . You are receiving this because you were mentioned.Message ID: @.***>

dafyre avatar Feb 15 '23 14:02 dafyre

@dafyre Many thx! Just merged it!

lemonmojo avatar Feb 15 '23 14:02 lemonmojo

You're welcome!

On Wed, Feb 15, 2023 at 9:48 AM Felix Deimel @.***> wrote:

@dafyre https://github.com/dafyre Many thx! Just merged it!

— Reply to this email directly, view it on GitHub https://github.com/royalapplications/toolbox/pull/38#issuecomment-1431487805, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADF74TVBROZPMSY2MSNZ4GTWXTUFNANCNFSM5TWBRYRQ . You are receiving this because you were mentioned.Message ID: @.***>

dafyre avatar Feb 15 '23 20:02 dafyre