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

Trusted Launch CLI Change Request - VM Disk Snapshot & Config #22275

Closed
AjKundnani opened this issue May 5, 2022 · 29 comments
Closed

Trusted Launch CLI Change Request - VM Disk Snapshot & Config #22275

AjKundnani opened this issue May 5, 2022 · 29 comments
Assignees
Labels
Auto-Assign Auto assign by bot Compute az vm/vmss/image/disk/snapshot feature-request

Comments

@AjKundnani
Copy link

AjKundnani commented May 5, 2022

Related command

az snapshot show
az disk create
az disk grant-access

Resource Provider

Microsoft.Compute/disks
Microsoft.Compute/snapshots

Description of Feature or Work Requested

feature request to add output value, new parameter and add values into existing parameters for following commands to support Trusted Launch VM Disks:

Feature request is to provide customers with completion of Trusted Launch VM (already GA) disk management:

  • validate if the snapshot customer has taken is enabled for TrustedLaunch.
  • import managed OS disk for Trusted Launch VMs. optionally along with VMGuestState disk if required.
  • upload managed OS disk with VM Guest State using new string parameter --upload-type which will replace existing parameter --for-upload

Additional Output Value

az snapshot show

When customer use az snapshot show command, the output should return SecurityProfile of the snapshot. SecurityProfile output is already supported in az disk show command, same needs to be extended to snapshots as well.

 "securityProfile": {
    "securityType": "TrustedLaunch"
  },

New Parameter

--security-data-uri (az disk create)

New string parameter --security-data-uri for command az disk create:

  • Allows customer to pass Blob URI for VM Guest State VHD. If customer does not use this parameter, DiskRP will create new VM Guest State (i.e., optional parameter)
  • When specified, the command should interpret that disk will be imported from un-managed VHD in storage account or another managed disk for TrustedLaunch VM OS Disk Security Type.
  • --security-type parameter mandatory when --security-data-uri value is passed.
  • --hyper-v-generation parameter value should be V2
  • This is the URI of a blob to be imported into VM guest state.
--upload-type (az disk create)

New string parameter --upload-type to replace --for-upload parameter in az disk create:

Accepted Value Description
Upload For Standard disk only upload scenario.
  • If used with TrustedLaunch --security-type, DiskRP will create new VM Guest State blob
  • Set Disk CreateOption to Upload in DiskRP API.
UploadWithSecurityData For OS Disk upload along with VM Guest State.
  • --security-type parameter mandatory
  • Set Disk CreateOption to UploadPreparedSecure in DiskRP API.
  • --hyper-v-generation parameter value should be V2
  • Parameter --secure-vm-guest-state-sas switched to true or ON for az disk grant-access
  • Not valid for data disk upload, only to be used for OS Disk upload at present.
--secure-vm-guest-state-sas (az disk grant-access)

New switch parameter --secure-vm-guest-state-sas for az disk grant-access:

  • Allows customer to query VM Guest State SAS.
  • Required when new parameter --Upload-Type is set to UploadWithSecurityData in az disk create command, which'll set CreateOption to UploadPreparedSecure
  • Output would show both accessSas and securityDataAccessSAS in response. If parameter not used, then output would return only accessSas

End to End Usage

Scenario 1 - Get Trusted Launch VM Disk Snapshot
  1. Get Virtual Machine Disk snapshot.

    az snapshot show --id $snapshotResourceId

    {
      "completionPercent": null,
      "creationData": {
        "createOption": "Copy",
        "galleryImageReference": null,
        "imageReference": null,
        "logicalSectorSize": null,
        "sourceResourceId": "/subscriptions/390a3e32-6963-47d8-bcef-ee8db1c22720/resourceGroups/tvm-cli-change-rg/providers/Microsoft.Compute/disks/tvm-cli-change-vm_OsDisk_1_78358962d0b645a0a3899f18c98b099a",
        "sourceUniqueId": "78358962-d0b6-45a0-a389-9f18c98b099a",
        "sourceUri": null,
        "storageAccountId": null,
        "uploadSizeBytes": null
      },
      "diskAccessId": null,
      "diskSizeBytes": 32213303296,
      "diskSizeGb": 30,
      "diskState": "Unattached",
      "encryption": {
        "diskEncryptionSetId": null,
        "type": "EncryptionAtRestWithPlatformKey"
      },
      "encryptionSettingsCollection": null,
      "extendedLocation": null,
      "hyperVGeneration": "V2",
      "id": "/subscriptions/390a3e32-6963-47d8-bcef-ee8db1c22720/resourceGroups/tvm-cli-change-rg/providers/Microsoft.Compute/snapshots/test01",
      "incremental": false,
      "location": "southeastasia",
      "managedBy": null,
      "name": "test01",
      "networkAccessPolicy": "AllowAll",
      "osType": "Linux",
      "provisioningState": "Succeeded",
      "publicNetworkAccess": "Enabled",
      "purchasePlan": null,
      "resourceGroup": "tvm-cli-change-rg",
      "securityProfile": {
        "securityType": "TrustedLaunch"
      },
      "sku": {
        "name": "Standard_LRS",
        "tier": "Standard"
      },
      "supportedCapabilities": {
        "acceleratedNetwork": true,
        "architecture": "x64"
      },
      "supportsHibernation": null,
      "tags": {},
      "timeCreated": "2022-04-29T12:48:54.475669+00:00",
      "type": "Microsoft.Compute/snapshots",
      "uniqueId": "8e845670-5c0f-4153-a178-9a544ba4b7e1"
    }
Scenario 2 - Secure Import of Trusted Launch VM OS Disk
  1. Create disk with --security-data-uri parameter:

    az disk create -n $diskName -g $resourceGroup \
        -l $location --os-type Windows --hyper-v-generation V2 \
        --security-type "TrustedLaunch" \
        --source $sourceDiskVhdUri --security-data-uri $guestStateDiskVhdUri \
        --sku standard_lrs
    
Scenario 3 - Secure Upload of Trusted Launch VM OS Disk
  1. Create an empty disk with --Upload-Type parameter:

    az disk create -n $diskName -g $resourceGroup \
        -l $location --os-type Windows --hyper-v-generation V2 \
        --security-type "TrustedLaunch" --Upload-Type "UploadWithSecurityData" \
        --upload-size-bytes 34359738880 --sku standard_lrs
    
  2. Grant access to generate accessSas and securityDataAccessSAS using --secure-vm-guest-state-sas parameter

    diskSas = $(az disk grant-access -n $diskName -g $resourceGroupName \
        --access-level Write --duration-in-seconds 86400 \
        --secure-vm-guest-state-sas)
    

    Returned value schema:

    {
      "accessSas": "https://md-impexp-t0rdsfgsdfg4.blob.core.windows.net/w2c3mj0ksfgl/abcd?sv=2017-04-17&sr=b&si=600a9281-d39e-4cc3-91d2-923c4a696537&sig=xXaT6mFgf139ycT87CADyFxb%2BnPXBElYirYRlbnJZbs%3D",
      "securityDataAccessSas": "<VM Guest State Sas URI>"
    }
  3. Copy Disk Content from Local Disk:

    AzCopy.exe copy "c:\somewhere\mydisk.vhd" $diskSas.AccessSAS --blob-type PageBlob

  4. Copy VM Guest State content from a local VHD:

    AzCopy.exe copy "c:\somewhere\myvmgs.vhd" $diskSas.securityDataAccessSAS --blob-type PageBlob

Minimum API Version Required

2021-08-01

Swagger PR link

Azure/azure-rest-api-specs#17118

Request Example

Target Date

2022-07-05

Additional context

Request for Trusted Launch VM feature.

Contacts

Role Contact
Main developer contacts (emails + github aliases) Abhishek Verma (AZURE) [email protected], Anshul Solanki [email protected]
PM contact (email + github alias) Ajay Kundnani [email protected]
Other people who should attend a design review (email) Run Cai [email protected], Deepak J V [email protected]
@ghost ghost added the Compute az vm/vmss/image/disk/snapshot label May 5, 2022
@ghost ghost added this to the Backlog milestone May 5, 2022
@ghost ghost assigned zhoxing-ms May 5, 2022
@ghost ghost added the Auto-Assign Auto assign by bot label May 5, 2022
@yonzhan yonzhan modified the milestones: Backlog, Jun 2022 (2022-07-05) May 5, 2022
@zhoxing-ms
Copy link
Contributor

@AjKundnani Hi, since our work in this sprint has been fully arranged, we don't have enough time to support more requirements. Can this feature request be postponed to the release of the next sprint (08-02)?

@AjKundnani
Copy link
Author

@zhoxing-ms - Sorry no, we're planning for GA of Confidential VM by June 30th 2022, so we request to consider this feature requirement for current sprint of 07-05 please.

@zhoxing-ms
Copy link
Contributor

diskSas = $(az disk grant-access -n $diskName -g $resourceGroupName
--access-level Write --duration-in-seconds 86400
--secure-vm-guest-state-sas true)

@AjKundnani I suggest that users use --secure-vm-guest-state-sas to indicate query securityDataAccessSAS instead of --secure-vm-guest-state-sas True, because there is no case for --secure-vm-guest-state-sas False, so the user's input should be simplified. What do you think of it?

@zhoxing-ms
Copy link
Contributor

zhoxing-ms commented May 31, 2022

Required when --for-upload is set to new value secureOSUpload.

@AjKundnani I concern that this design will lead to breaking change, because if the create option of disk queried in the CLI scripts are secureOSUpload, the execution will fail due to the --secure-vm-guest-state-sas parameter is an additional required parameter.
Could we take --secure-vm-guest-state-sas as the default behavior when the create option of disk is secureOSUpload?

@AjKundnani
Copy link
Author

diskSas = $(az disk grant-access -n $diskName -g $resourceGroupName
--access-level Write --duration-in-seconds 86400
--secure-vm-guest-state-sas true)

@AjKundnani I suggest that users use --secure-vm-guest-state-sas to indicate query securityDataAccessSAS instead of --secure-vm-guest-state-sas True, because there is no case for --secure-vm-guest-state-sas False, so the user's input should be simplified. What do you think of it?

@zhoxing-ms - Agreed, we can use it as a switch rather than bool parameter.

@AjKundnani
Copy link
Author

--secure-vm-guest-state-sas

@zhoxing-ms - As discussed, CLI will query disk resources created using az disk create command and if --for-upload is set to secureOsUpload then --secure-vm-guest-state-sas switch will be auto-added in az disk grant-access command.

Additionally, --for-upload parameter will continue to support true as alias for nonSecureOsUpload with warning message that true will be deprecated in future and preferably with link where end users can read about difference between secureOsUpload and nonSecureOsUpload.

@zhoxing-ms
Copy link
Contributor

@AjKundnani The az snapshot create command also has the --for-upload parameter and similar parameter logic.
Do we also need to consider supporting nonSecureOSUpload and secureOSUpload of --for-upload for the az snapshot create command in the same way?

@AjKundnani
Copy link
Author

@AjKundnani The az snapshot create command also has the --for-upload parameter and similar parameter logic. Do we also need to consider supporting nonSecureOSUpload and secureOSUpload of --for-upload for the az snapshot create command in the same way?

@zhoxing-ms - Currently we do not have plans to support SecureOSUpload scenario for snapshots. I am checking more with team and if needed will setup new request for az snapshot create command.

cc @lakmeedee

@zhoxing-ms
Copy link
Contributor

zhoxing-ms commented Jun 8, 2022

false | OS Disk created is not using upload method.

@AjKundnani Actually, the false value seems to have no practical effect on --for-upload. When users do not pass in the parameter --for-upload, it already represents OS Disk created is not using upload method, so there is no need to pass in the extra parameter --for-upload false, right?

If so, I suggest we could add a new parameter --upload-type to support the nonSecureOSUpload and secureOSUpload.
For the original --for-upload parameter, we can add the deprecate_info=self.deprecate(redirect='--upload-type', hide=True)) to it to hide the deprecated parameter in the help message and prompt users that: this parameter will be deprecated in the future, for the use case of using --for-upload true, please use --upload-type nonSecureOSUpload instead. Moreover, this solution will not affect the user's continued usage of --for-upload parameter, so this will not lead to breaking change

The benefits for this solution are:

  1. It simplifies the valid values of parameters and avoids the possible confusion caused by the non juxtaposition between secureOSUpload , nonSecureOSUpload and false
  2. It hides the deprecated parameters in the help message, so as to reduce the learning burden for new users
  3. More upload types can be smoothly expanded in the future

What do you think of it?

@AjKundnani
Copy link
Author

AjKundnani commented Jun 8, 2022

false | OS Disk created is not using upload method.

@AjKundnani Actually, the false value seems to have no practical effect on --for-upload. When users do not pass in the parameter --for-upload, it already represents OS Disk created is not using upload method, so there is no need to pass in the extra parameter --for-upload false, right?

If so, I suggest we could add a new parameter --upload-type to support the nonSecureOSUpload and secureOSUpload. For the original --for-upload parameter, we can add the deprecate_info=self.deprecate(redirect='--upload-type', hide=True)) to it to hide the deprecated parameter in the help message and prompt users that: this parameter will be deprecated in the future, for the use case of using --for-upload true, please use --upload-type nonSecureOSUpload instead. Moreover, this solution will not affect the user's continued usage of --for-upload parameter, so this will not lead to breaking change

The benefits for this solution are:

  1. It simplifies the valid values of parameters and avoids the possible confusion caused by the non juxtaposition between secureOSUpload , nonSecureOSUpload and false
  2. It hides the deprecated parameters in the help message, so as to reduce the learning burden for new users
  3. More upload types can be smoothly expanded in the future

What do you think of it?

@zhoxing-ms - false value for --for-upload parameter is being carried forward from existing az disk create command. If users do not specify --for-upload the default value is false currently.

For upload type, I am discussing internally what would be best option, will discuss proposed solution as well and get back on same.

@AjKundnani
Copy link
Author

false | OS Disk created is not using upload method.

@AjKundnani Actually, the false value seems to have no practical effect on --for-upload. When users do not pass in the parameter --for-upload, it already represents OS Disk created is not using upload method, so there is no need to pass in the extra parameter --for-upload false, right?

If so, I suggest we could add a new parameter --upload-type to support the nonSecureOSUpload and secureOSUpload. For the original --for-upload parameter, we can add the deprecate_info=self.deprecate(redirect='--upload-type', hide=True)) to it to hide the deprecated parameter in the help message and prompt users that: this parameter will be deprecated in the future, for the use case of using --for-upload true, please use --upload-type nonSecureOSUpload instead. Moreover, this solution will not affect the user's continued usage of --for-upload parameter, so this will not lead to breaking change

The benefits for this solution are:

  1. It simplifies the valid values of parameters and avoids the possible confusion caused by the non juxtaposition between secureOSUpload , nonSecureOSUpload and false
  2. It hides the deprecated parameters in the help message, so as to reduce the learning burden for new users
  3. More upload types can be smoothly expanded in the future

What do you think of it?

@zhoxing-ms - After internal discussion and understanding more from DiskRP team, I've edited the request to include new parameter for-upload-with-security-data in the request. No edit or modification needed to for-upload parameter at this point.

--secure-vm-guest-state-sas can be switched on for az disk grant-access command when CreateOption for disk is set to UploadpreparedSecure using --for-upload-with-security-data parameter. If user uploads using --for-upload parameter, then CreateOption will be set to Upload.

Only one of --for-upload or --for-upload-with-security-data can be used, not both together.

Hope this clarifies and simplifies request and ensures good end user experience.

@chasewilson
Copy link
Contributor

This is an improvement and along the lines of the issues I saw with the original proposal.

However, there is one issue I have. We are looking at adding another Parameter to a command that already has appx 35 parameters. Is it possibnle to use an existing parameter to accomplish this?

Currently, there is a --security-type parameter that accepts a value of TrustedLaunch. Is there a way to expand this one rather than creating a brand new paramter? E.G.

az disk create --for-upload --security-type TrustedLaunch

With the combination of --for-upload and --security-type, shouldn't that be a definitive call to meet the disired result here?

  1. This will keep the user experience the same and reduce required changes.
  2. This is simpler than adding a similar paramter to an already crowded command.
  3. If there are more security types in the future, this is also a pather forward for expansion without user impact.

Hope this helps. Let me know if anything isn't clear or if you disagree on any of my points. I'm open to discussion here and would like to see a minimal amount of customer friction, added complexity, or boxing ourselves into a certain corner we will have to break ourselves out of in the future and hurt the customer experience.

@AjKundnani
Copy link
Author

This is an improvement and along the lines of the issues I saw with the original proposal.

However, there is one issue I have. We are looking at adding another Parameter to a command that already has appx 35 parameters. Is it possibnle to use an existing parameter to accomplish this?

Currently, there is a --security-type parameter that accepts a value of TrustedLaunch. Is there a way to expand this one rather than creating a brand new paramter? E.G.

az disk create --for-upload --security-type TrustedLaunch

With the combination of --for-upload and --security-type, shouldn't that be a definitive call to meet the disired result here?

  1. This will keep the user experience the same and reduce required changes.
  2. This is simpler than adding a similar paramter to an already crowded command.
  3. If there are more security types in the future, this is also a pather forward for expansion without user impact.

Hope this helps. Let me know if anything isn't clear or if you disagree on any of my points. I'm open to discussion here and would like to see a minimal amount of customer friction, added complexity, or boxing ourselves into a certain corner we will have to break ourselves out of in the future and hurt the customer experience.

@chasewilson, the limitation in this approach is when end user or customer would upload the VM Guest State Blob

  • To support uploaded VM Guest State Blob, create option should be UploadPreparedSecure, this is achieved through new parameter --for-upload-with-security-data
  • To generate default or blank VM Guest State Blob, create option should be Upload, this is achieved through existing parameter --for-upload. i,e. with Trusted Launch, its not mandatory for end user to upload VM Guest State Blob
  • As ConfidentialVM security type will mature, there'll be scenarios where VM Guest State Blob will be mandatory i.e. DiskRP will not auto-generate the VMGS blob and end user will have to upload same. In those scenarios create option will need to be mandatorily set to UploadPreparedSecure

@chasewilson
Copy link
Contributor

Ok team, here's a thought. What if we adjusted it to something like this:

az disk create --for-upload UploadPreparedSecure/VMGS # or some value along those lines to indicate the UploadPreparedSecure value

In the above example, we adjust the current --for-upload parameter from a switch to accepting values. To avoid breaking changes we could have the default set to standard or something to represent the blank state. (This logic would have to be worked to try and avoid braking changes. and can be discussed.)

Benefits

  • This will alleviate the increase in complexity
  • This meets the criteria to provide either either a blank value or the UploadPreparedSecure for the mandatory VMGS Blob
  • Allows us to expdand in the future if necessary without adding parameters for specific instances.

Concerns

  • the implementation of --for-upload will have to be thought out planned to try and avoid breaking changes.
    • We could potentially adjust as far as necessary in a future breaking change release if necessary
  • Does this work with the requirements from the eng teams?

@zhoxing-ms @AjKundnani let me know what you think and thank you both for working on this so patiently with me.

@zhoxing-ms
Copy link
Contributor

@chasewilson In fact, this idea is similar to the original design. The reason why we want to modify the design of using the --for-upload parameter is mainly based on the following considerations:

  1. In order to avoid breaking change, we need to support both the original values of true,false and the new values UploadPreparedSecure,UploadWithoutSecure to be added in a future period of time. These parameters are not aligned, which may confuse users.
  2. Currently, CLI telemetry does not record parameter values, so we cannot judge when to completely remove those deprecated values by observing the Telemetry data.
  3. The deprecation of parameter values can only be prompted through the warning log. There is no standard process and eye-catching prompt like the deprecation of parameters.

So, inspired by your idea, I have this proposal: if we want to avoid adding additional parameters to increase complexity, perhaps we can consider hiding the original parameter and deprecating it in the future, and adding a new parameter to support UploadPreparedSecure and UploadWithoutSecure.
This solution allows the new parameter to keep only the clearly aligned new parameter values which reduces user confusion about parameter values. Moreover, there is a standard deprecation process for deprecated parameters. We can decide when to completely remove the deprecated parameter --for-upload through the users' usage data from CLI Telemetry.
For more details, please refer to my previous comments #22275 (comment)

@chasewilson @AjKundnani What do you think of this solution? If you have any ideas or suggestions, I'm glad to hear them

@chasewilson
Copy link
Contributor

Hey @zhoxing-ms, I think that this is a good compromise and solid path forward! @AjKundnani, what do you think?

For anyone here, if you have ideas on parameter names feel free to throw them out there.

@zhoxing-ms
Copy link
Contributor

@AjKundnani I have two more questions to confirm with you:

  1. For --upload-type UploadWithSecurityData, you mentioned Not valid for data disk upload, only to be used for OS Disk upload at present.
    Do we need to add local verification to determine whether the disk is OS disk?
    Or does the REST service return an appropriate error message to prompt users, so we can directly display the error message from service? (if data disk is supported in the future, it can take effect on the CLI side in time)

  2. For --secure-vm-guest-state-sas, you mentioned required only if customer intends to upload VM Guest State. If customer does not uploads VM Guest State, DiskRP will create new VM Guest State blob.
    Do we need to add local verification for this case so that the --secure-vm-guest-state-sas parameter can only be passed in when the disk is used for uploading VM Guest State?

@AjKundnani
Copy link
Author

@AjKundnani I have two more questions to confirm with you:

  1. For --upload-type UploadWithSecurityData, you mentioned Not valid for data disk upload, only to be used for OS Disk upload at present.
    Do we need to add local verification to determine whether the disk is OS disk?
    Or does the REST service return an appropriate error message to prompt users, so we can directly display the error message from service? (if data disk is supported in the future, it can take effect on the CLI side in time)
  2. For --secure-vm-guest-state-sas, you mentioned required only if customer intends to upload VM Guest State. If customer does not uploads VM Guest State, DiskRP will create new VM Guest State blob.
    Do we need to add local verification for this case so that the --secure-vm-guest-state-sas parameter can only be passed in when the disk is used for uploading VM Guest State?

@zhoxing-ms

  1. At present, Data disk do not support VMGuestState, REST service should return appropriate error, let me know if you need specific error message? Currently we do not have fixed plan to support Data Disk VMGS in future, so based on CLI best practices, kindly add or skip local verification as needed.
  2. As per our initial discussion, --secure-vm-guest-state-sas for command az disk grant-access should be auto turned-on or added into command if the CreateOption for disk is set to UploadPreparedSecure. I will update the description for same to align with updated request. It's not required when CreateOption is set to Upload.

@zhoxing-ms
Copy link
Contributor

At present, Data disk do not support VMGuestState, REST service should return appropriate error, let me know if you need specific error message? Currently we do not have fixed plan to support Data Disk VMGS in future, so based on CLI best practices, kindly add or skip local verification as needed.

@AjKundnani Could you show me the error messages from REST service? If the error message returned by service is appropriate and clear enough, I prefer to use the error message from service to avoid introducing complex judgment logic in client side.

As per our initial discussion, --secure-vm-guest-state-sas for command az disk grant-access should be auto turned-on or added into command if the CreateOption for disk is set to UploadPreparedSecure. I will update the description for same to align with updated request. It's not required when CreateOption is set to Upload.

Therefore, when CreateOption is UploadPreparedSecure, the --secure-vm-guest-state-sas will be turned on for az disk grant-access automatically. When CreateOption is Upload, --secure-vm-guest-state-sas becomes an optional parameter. Is my understanding right now?

@AjKundnani
Copy link
Author

AjKundnani commented Jun 27, 2022

At present, Data disk do not support VMGuestState, REST service should return appropriate error, let me know if you need specific error message? Currently we do not have fixed plan to support Data Disk VMGS in future, so based on CLI best practices, kindly add or skip local verification as needed.

@AjKundnani Could you show me the error messages from REST service? If the error message returned by service is appropriate and clear enough, I prefer to use the error message from service to avoid introducing complex judgment logic in client side.

As per our initial discussion, --secure-vm-guest-state-sas for command az disk grant-access should be auto turned-on or added into command if the CreateOption for disk is set to UploadPreparedSecure. I will update the description for same to align with updated request. It's not required when CreateOption is set to Upload.

Therefore, when CreateOption is UploadPreparedSecure, the --secure-vm-guest-state-sas will be turned on for az disk grant-access automatically. When CreateOption is Upload, --secure-vm-guest-state-sas becomes an optional parameter. Is my understanding right now?

@zhoxing-ms -

  1. Correction, I cross-checked with DiskRP team, currently API does not differentiate between OS and data disk, so there'll be no error from API, will request to restrict it from client side as we do not have plan for using VMGS with data disk at present.
  2. If the CreateOption is set to Upload then parameter --secure-vm-guest-state-sas should not be allowed, it should only be used for scenario where CreateOption is UploadPreparedSecure.

@zhoxing-ms
Copy link
Contributor

Correction, I cross-checked with DiskRP team, currently API does not differentiate between OS and data disk, so there'll be no error from API, will request to restrict it from client side as we do not have plan for using VMGS with data disk at present.

@AjKundnani In fact, if there are no related restrictions on REST service, it is unreasonable to only add restrictions on CLI side. This cannot solve the problems of users who directly call the REST and users who use other clients (such as: PowerShell, Terraform, Python SDK...) Will the rest service add related restrictions later?

If the CreateOption is set to Upload then parameter --secure-vm-guest-state-sas should not be allowed, it should only be used for scenario where CreateOption is UploadPreparedSecure.

If so, the --secure-vm-guest-state-sas parameter does not need to be exposed to users, because --secure-vm-guest-state-sas will be turned on for az disk grant-access automatically when CreateOption is UploadPreparedSecure.
Do we choose not to expose the --secure-vm-guest-state-sas parameter to users? Or do we cancel the auto turned on behavior for UploadPreparedSecure and let users manually enter the --secure-vm-guest-state-sas parameter, but do not allow them to enter --secure-vm-guest-state-sas when CreateOption is Upload?

@zhoxing-ms
Copy link
Contributor

zhoxing-ms commented Jun 27, 2022

@AjKundnani Besides, I have another question: are the verification logics in the below two places applicable to --upload-type UploadWithSecurityData as well as --upload-type Upload (Upload corresponds to the original for_upload)?

  1. --upload-size-bytes should be used together with --for-upload
    code link:

    if upload_size_bytes is not None and for_upload is not True:
    raise CLIError('usage error: --upload-size-bytes should be used together with --for-upload')

  2. --size-gb or --upload-size-bytes required to create an empty disk
    code link:

    if size_gb is None and upload_size_bytes is None and (option == DiskCreateOption.empty or for_upload):
    raise CLIError('usage error: --size-gb or --upload-size-bytes required to create an empty disk')

@AjKundnani
Copy link
Author

@AjKundnani Besides, I have another question: are the verification logics in the below two places applicable to --upload-type UploadWithSecurityData as well as --upload-type Upload (Upload corresponds to the original for_upload)?

  1. --upload-size-bytes should be used together with --for-upload
    code link:
    if upload_size_bytes is not None and for_upload is not True:
    raise CLIError('usage error: --upload-size-bytes should be used together with --for-upload')
  2. --size-gb or --upload-size-bytes required to create an empty disk
    code link:
    if size_gb is None and upload_size_bytes is None and (option == DiskCreateOption.empty or for_upload):
    raise CLIError('usage error: --size-gb or --upload-size-bytes required to create an empty disk')

@zhoxing-ms - As per Disk RP team "uploadpreparedsecure has same verifications which are applicable to upload create option"

@AjKundnani
Copy link
Author

Correction, I cross-checked with DiskRP team, currently API does not differentiate between OS and data disk, so there'll be no error from API, will request to restrict it from client side as we do not have plan for using VMGS with data disk at present.

@AjKundnani In fact, if there are no related restrictions on REST service, it is unreasonable to only add restrictions on CLI side. This cannot solve the problems of users who directly call the REST and users who use other clients (such as: PowerShell, Terraform, Python SDK...) Will the rest service add related restrictions later?

If the CreateOption is set to Upload then parameter --secure-vm-guest-state-sas should not be allowed, it should only be used for scenario where CreateOption is UploadPreparedSecure.

If so, the --secure-vm-guest-state-sas parameter does not need to be exposed to users, because --secure-vm-guest-state-sas will be turned on for az disk grant-access automatically when CreateOption is UploadPreparedSecure. Do we choose not to expose the --secure-vm-guest-state-sas parameter to users? Or do we cancel the auto turned on behavior for UploadPreparedSecure and let users manually enter the --secure-vm-guest-state-sas parameter, but do not allow them to enter --secure-vm-guest-state-sas when CreateOption is Upload?

@zhoxing-ms

  1. I can check but currently there's no plan that am aware of to add this restriction. If its not reasonable as per CLI best practices, we can skip this check. Will need to ensure this restriction is published in docs such that end user is aware of this restriction.
  2. For seamless user experience, if we can auto-turn flag --secure-vm-guest-state-sas on based on Disk CreateOption of UploadPreparedSecure, that will be ideal and will not need to expose this parameter to end user. Because VMGS is mandatory when CreateOption is UploadPreparedSecure and not required when CreateOption is Upload.

@zhoxing-ms
Copy link
Contributor

zhoxing-ms commented Jun 27, 2022

As per Disk RP team "uploadpreparedsecure has same verifications which are applicable to upload create option"

@AjKundnani I see. Thanks for your confirmation!

I can check but currently there's no plan that am aware of to add this restriction. If its not reasonable as per CLI best practices, we can skip this check. Will need to ensure this restriction is published in docs such that end user is aware of this restriction.

We'd better not only add this verification on the CLI side, otherwise it may be inconsistent with the experience of other sides. We may consider adding this restriction information to the help message to let users aware of this restriction. (it will be automatically synchronized to our public doc)

For seamless user experience, if we can auto-turn flag --secure-vm-guest-state-sas on based on Disk CreateOption of UploadPreparedSecure, that will be ideal and will not need to expose this parameter to end user. Because VMGS is mandatory when CreateOption is UploadPreparedSecure and not required when CreateOption is Upload.

I see. It sounds like a good solution not to expose the parameter --secure-vm-guest-state-sas to users

@AjKundnani
Copy link
Author

As per Disk RP team "uploadpreparedsecure has same verifications which are applicable to upload create option"

@AjKundnani I see. Thanks for your confirmation!

I can check but currently there's no plan that am aware of to add this restriction. If its not reasonable as per CLI best practices, we can skip this check. Will need to ensure this restriction is published in docs such that end user is aware of this restriction.

We'd better not only add this verification on the CLI side, otherwise it may be inconsistent with the experience of other sides. We may consider adding this restriction information to the help message to let users aware of this restriction. (it will be automatically synchronized to our public doc)

For seamless user experience, if we can auto-turn flag --secure-vm-guest-state-sas on based on Disk CreateOption of UploadPreparedSecure, that will be ideal and will not need to expose this parameter to end user. Because VMGS is mandatory when CreateOption is UploadPreparedSecure and not required when CreateOption is Upload.

I see. It sounds like a good solution not to expose the parameter --secure-vm-guest-state-sas to users

Thanks @zhoxing-ms - Adding help message about restriction of OS disk only for VMGS should be helpful.

For parameter --secure-vm-guest-state-sas, if we can publish example in docs on how to use the returned VM Guest State SAS URL for uploading disk with security data, that'll be helpful.

@zhoxing-ms
Copy link
Contributor

For parameter --secure-vm-guest-state-sas, if we can publish example in docs on how to use the returned VM Guest State SAS URL for uploading disk with security data, that'll be helpful.

@AjKundnani Could you give me a good example? I will consider adding it in an appropriate way

@Jing-song
Copy link
Contributor

Jing-song commented Jun 29, 2022

Hi @AjKundnani
When I test Create disk with --security-data-uri parameter, how to get $sourceDiskVhdUri,$guestStateDiskVhdUri.

az disk create -n $diskName -g $resourceGroup \
    -l $location --os-type Windows --hyper-v-generation V2 \
    --security-type "TrustedLaunch" \
    --source $sourceDiskVhdUri --security-data-uri $guestStateDiskVhdUri \
    --sku standard_lrs

when I use strorage contaniner blob as uri for test, if use --source, createoption will use copy, TrustedLaunch not support for copy, if don't use --source, serivce return (InternalOperationError) An internal error occurred in the operation.
Should I refer to this example to create it? if so, do we need to add some new parameters (such as sourceUri)?
image

@AjKundnani
Copy link
Author

Correction, I cross-checked with DiskRP team, currently API does not differentiate between OS and data disk, so there'll be no error from API, will request to restrict it from client side as we do not have plan for using VMGS with data disk at present.

@AjKundnani In fact, if there are no related restrictions on REST service, it is unreasonable to only add restrictions on CLI side. This cannot solve the problems of users who directly call the REST and users who use other clients (such as: PowerShell, Terraform, Python SDK...) Will the rest service add related restrictions later?

If the CreateOption is set to Upload then parameter --secure-vm-guest-state-sas should not be allowed, it should only be used for scenario where CreateOption is UploadPreparedSecure.

If so, the --secure-vm-guest-state-sas parameter does not need to be exposed to users, because --secure-vm-guest-state-sas will be turned on for az disk grant-access automatically when CreateOption is UploadPreparedSecure. Do we choose not to expose the --secure-vm-guest-state-sas parameter to users? Or do we cancel the auto turned on behavior for UploadPreparedSecure and let users manually enter the --secure-vm-guest-state-sas parameter, but do not allow them to enter --secure-vm-guest-state-sas when CreateOption is Upload?

@zhoxing-ms

  1. I can check but currently there's no plan that am aware of to add this restriction. If its not reasonable as per CLI best practices, we can skip this check. Will need to ensure this restriction is published in docs such that end user is aware of this restriction.
  2. For seamless user experience, if we can auto-turn flag --secure-vm-guest-state-sas on based on Disk CreateOption of UploadPreparedSecure, that will be ideal and will not need to expose this parameter to end user. Because VMGS is mandatory when CreateOption is UploadPreparedSecure and not required when CreateOption is Upload.

@zhoxing-ms @PARADISSEEKR - Based on our conversation about Confidential VM, new scenario has come out where end users might need to export the VMGS vhd file. If we can expose the parameter to --secure-vm-guest-state-sas end users please?

  • If CreateOption is UploadPreparedSecure, the parameter --secure-vm-guest-state-sas will get auto-set or turned on for command az disk grant-access
  • Else it will be optional parameter which users can use if they need to export the VMGS vhd file using generated SAS URL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Auto-Assign Auto assign by bot Compute az vm/vmss/image/disk/snapshot feature-request
Projects
None yet
Development

No branches or pull requests

5 participants