Skip to content

Update Set-AzureRmVMDiskEncryptionExtension.md #6100

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

Merged
merged 2 commits into from
May 3, 2018

Conversation

mestew
Copy link
Contributor

@mestew mestew commented May 3, 2018

I happened to be testing a similar script and think I ran across a few small issues with the examples.

  1. $KEKName not set in examples 3&4
  2. New-AzureRmADApplication seems to use -CertValue rather than -KeyValue and -KeyType https://docs.microsoft.com/en-us/powershell/module/azurerm.resources/new-azurermadapplication?view=azurermps-5.7.0
  3. Looks like the KEK bit is missing from Set-AzureRmVMDiskEncryptionExtension in Example 4 for cert + KEK.

Description

Checklist

I happened to be testing a similar script and think I ran across a few small issues with the examples. 
1. $KEKName not set in examples 3&4
2. New-AzureRmADApplication seems to use -CertValue rather than -KeyValue and -KeyType  https://docs.microsoft.com/en-us/powershell/module/azurerm.resources/new-azurermadapplication?view=azurermps-5.7.0 
3. Looks like the KEK bit is missing from Set-AzureRmVMDiskEncryptionExtension in Example 4 for cert + KEK.
@azuresdkci
Copy link

Can one of the admins verify this patch?

@mestew
Copy link
Contributor Author

mestew commented May 3, 2018

I just realized I stuck this in the preview branch rather than master (oops, I didn't verify, sorry). Let me know if you want me to close this PR and just do a new one against master

@cormacpayne
Copy link
Member

@azuresdkci add to whitelist

@cormacpayne
Copy link
Member

@mestew Hey Meghan, thanks for creating this PR 😀 I pushed a commit to your fork/branch that updates another Set-AzureRmVMDiskEncryptionExtension.md file that we have in our repository. Our default branch is preview, so your did the right thing opening the PR against that branch.

@ejarvi Hey Eric, would you mind reviewing the change made to this markdown file when you get the chance?

@ejarvi
Copy link
Contributor

ejarvi commented May 3, 2018

Yes - reviewed the changes and they look correct, thank you Meghan for catching this.

@ejarvi ejarvi closed this May 3, 2018
@ejarvi ejarvi reopened this May 3, 2018
@ejarvi ejarvi assigned cormacpayne and unassigned ejarvi May 3, 2018
@cormacpayne cormacpayne removed their assignment May 3, 2018
@cormacpayne cormacpayne merged commit 87c9dd8 into Azure:preview May 3, 2018
@mestew mestew deleted the patch-1 branch May 4, 2018 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants