Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added scripts to perform copy-keys operation. #1

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

Conversation

punit-bhatt
Copy link

Two scripts have been added:

Create-aad-app: Creates a new aad app and secret. Or a new secret for existing aad app
copy-keys: Transfer BEK and wrapped BEK to the other key vault

@punit-bhatt
Copy link
Author

Added a new script taking care of the entire GUI.

Things done - GUI, different validations, Key Vault creations as requested with default names.

Things NOT done - handling common parameter inputs and GUI inputs together, still using AAD app for token generation, informative comments.

@punit-bhatt
Copy link
Author

punit-bhatt commented Aug 17, 2018

  • Outputs have been changed to what was required.
  • Source and Target key vaults of both – bek and kek, are being checked to ensure min. required permissions
  • All policies are being copied from src to “newly created” target vaults. Since this tends to be a time consuming process, it happens towards the end and a progress bar is shown to let user know
  • Default target KEK and BEK key vaults are shown with/without “(new)”.
    o This process involves going through the vm list till you get first bek and first kek.
    o Whichever is not found, is marked as Not Applicable. If both not found, the select button is disabled as well.
    o These are used to check if target vaults exist.
    o Takes a bit of time because of resource calls made to traverse through vms and above point
  • Hovering over enabled option labels shows messages. Right now these messages are meaningless, need to be edited as required.
  • Loading message and regions list updated to keep things in parallel with the portal

I have tested quite a bit but do let me know if you come across any failures or scenarios not handled.

A bek+kek test case :
Subscription: ASR Canary Test Subscription 1
Resource Group: test05pb
Vm: testvm05pb02 ( I think the other 2 vms are as well, will check and let you know)

Src bek vault: testkv05pb
Src kek vault: testkv05pb

Target vault has been deleted, for now.


[Parameter(Mandatory = $true,

HelpMessage="Identifier of the Azure subscription to be used")]
Copy link
Member

Choose a reason for hiding this comment

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

Identifier?


[Parameter(Mandatory = $true,

HelpMessage="Name of the AAD application that will be used to write secrets to KeyVault. A new application with this name will be created if one doesn't exist. If this app already exists, pass aadClientSecret parameter to the script")]
Copy link
Member

Choose a reason for hiding this comment

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

From help message: "pass aadClientSecret parameter to the script"
are we suggesting customers to pass aadClientSecret with aadAppName?


# AAD app wasn't created

Write-Error "Failed to create AAD app $aadAppName. Please log in to Azure using Connect-AzureRmAccount and try again";
Copy link
Member

Choose a reason for hiding this comment

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

Will "Please log in to Azure using Connect-AzureRmAccount and try again" be the only reason for failure?
If so, why don't we checkin in the start and run the flow. If needed, we can even check the subscription context and run then login cmdlet asking for the context.

@sriramvu
Copy link
Member

  1. Please remove unnecesary new lines.
  2. Maintain 100 column restriction:
  • try having statement in single line if it fits in 100 column, else break it down into multiple lines with consistent identitation.
  1. Summary lines
  2. Avoid multiple white spaces.
  3. Avoid semicolon: generally PowerShell refers new lines as statement seperators (semicolons may be used for multiple statements on a single line).
  4. Use Set-StrictMode
  5. Refer input parameters with PSBoundParameters
  6. Use #Requires if you have taken dependency on any particular version or so: remember we were discussing one of the options were not available in one of our installed MSIs.
  7. Add usage.
  8. Remove commented not required code.

@sriramvu
Copy link
Member

  1. Rather than single file, better break down to multiple files (main flow, helper, UI, ...).

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

Successfully merging this pull request may close these issues.

2 participants