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

compute VirtualMachines_ListAll OBJECT_MISSING_REQUIRED_PROPERTY #14459

Closed
ctaggart opened this issue May 18, 2021 · 11 comments
Closed

compute VirtualMachines_ListAll OBJECT_MISSING_REQUIRED_PROPERTY #14459

ctaggart opened this issue May 18, 2021 · 11 comments
Assignees
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. Compute - Extensions Compute - VM Service Attention Workflow: This issue is responsible by Azure service team.

Comments

@ctaggart
Copy link
Contributor

The latest specs for compute #14223 #12334 have Swagger correctness bugs that break deserialization with the Azure SDK for Rust Azure/azure-sdk-for-rust#54. In this case, it looks to be a bug in the Swagger spec. The API does not send back the required location.. The definition should probably be SubResourceReadOnly instead of Resource. See #14458.

@ghost ghost added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label May 18, 2021
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label May 19, 2021
@Sandido Sandido added Compute - Extensions needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. labels May 19, 2021
@JackTn JackTn added Service Attention Workflow: This issue is responsible by Azure service team. and removed needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. labels May 27, 2021
@ghost
Copy link

ghost commented May 27, 2021

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @Drewm3, @amjads1.

Issue Details

The latest specs for compute #14223 #12334 have Swagger correctness bugs that break deserialization with the Azure SDK for Rust Azure/azure-sdk-for-rust#54. In this case, it looks to be a bug in the Swagger spec. The API does not send back the required location.. The definition should probably be SubResourceReadOnly instead of Resource. See #14458.

Author: ctaggart
Assignees: akning-ms, Sandido
Labels:

Compute - Extensions, Compute - VM, Service Attention

Milestone: -

@Vaibhav-Agar
Copy link

@avirishuv can you please have a look?

@avirishuv
Copy link

Following up to investigate this.

@avirishuv avirishuv added the question The issue doesn't require a change to the product in order to be resolved. Most issues start as that label Jun 15, 2021
@avirishuv avirishuv self-assigned this Jun 15, 2021
@avirishuv
Copy link

quick update: this is added to the backlog for bugfix, will provide an update here when it gets picked up.

@avirishuv avirishuv added the bug This issue requires a change to an existing behavior in the product in order to be resolved. label Jul 22, 2021
@avirishuv avirishuv removed the question The issue doesn't require a change to the product in order to be resolved. Most issues start as that label Jul 22, 2021
@avirishuv
Copy link

quick update: the fix is still in progress.

@avirishuv
Copy link

fix rollout is yet to be completed for this issue.

@cataggar
Copy link
Member

I'm able to reproduce the api-version 2021-11-01. I'll try to break down the problem a bit more. The operation VirtualMachines_ListAll returns a list of VirtualMachine definitions.

"VirtualMachine": {
"properties": {
"plan": {
"$ref": "#/definitions/Plan",
"description": "Specifies information about the marketplace image used to create the virtual machine. This element is only used for marketplace images. Before you can use a marketplace image from an API, you must enable the image for programmatic use. In the Azure portal, find the marketplace image that you want to use and then click **Want to deploy programmatically, Get Started ->**. Enter any required information and then click **Save**."
},
"properties": {
"x-ms-client-flatten": true,
"$ref": "#/definitions/VirtualMachineProperties"
},
"resources": {
"readOnly": true,
"type": "array",
"items": {
"$ref": "#/definitions/VirtualMachineExtension"
},
"description": "The virtual machine child extension resources."
},

Each VirtualMachine has a resources element that contains an array of VirtualMachineExtensionImage:

"VirtualMachineExtensionImage": {
"properties": {
"properties": {
"x-ms-client-flatten": true,
"$ref": "#/definitions/VirtualMachineExtensionImageProperties"
}
},
"required": [
"name",
"location"
],
"allOf": [
{
"$ref": "#/definitions/Resource"
}
],
"description": "Describes a Virtual Machine Extension Image."
},

Use your favorite tool or SDK to list VMs. An example request is
GET /subscriptions/7f1fae41-7708-4fa4-89b3-f6552cad2fc1/providers/Microsoft.Compute/virtualMachines?api-version=2021-11-01

One of the resources starts off like this:

      "resources": [
        {
          "id": "/subscriptions/7f1fae41-7708-4fa4-89b3-f6552cad2fc1/resourceGroups/AAGILLCS/providers/Microsoft.Compute/virtualMachines/tnt42-mgmt-p01-eastus2-incumbentVm/extensions/Microsoft.Azure.Monitor.AzureMonitorLinuxAgent"
        },
        {
          "id": "/subscriptions/7f1fae41-7708-4fa4-89b3-f6552cad2fc1/resourceGroups/AAGILLCS/providers/Microsoft.Compute/virtualMachines/tnt42-mgmt-p01-eastus2-incumbentVm/extensions/Microsoft.Azure.Security.Monitoring.AzureSecurityLinuxAgent"
        }

Notice that it only includes the id field. That does not match this definition:

    "Resource": {
      "description": "The Resource model definition.",
      "properties": {
        "id": {
          "readOnly": true,
          "type": "string",
          "description": "Resource Id"
        },
        "name": {
          "readOnly": true,
          "type": "string",
          "description": "Resource name"
        },
        "type": {
          "readOnly": true,
          "type": "string",
          "description": "Resource type"
        },
        "location": {
          "type": "string",
          "description": "Resource location"
        },
        "tags": {
          "type": "object",
          "additionalProperties": {
            "type": "string"
          },
          "description": "Resource tags"
        }
      },
      "required": [
        "location"
      ],
      "x-ms-azure-resource": true
    },

A location field is required for a Resource. I think the implementation actually matches a SubResourceReadOnly:

    "SubResourceReadOnly": {
      "properties": {
        "id": {
          "readOnly": true,
          "type": "string",
          "description": "Resource Id"
        }
      },
      "x-ms-azure-resource": true
    },

@grizzlytheodore grizzlytheodore self-assigned this Mar 16, 2022
@grizzlytheodore
Copy link
Member

Assigned myself this, as I am taking ownership of this issue

@grizzlytheodore
Copy link
Member

grizzlytheodore commented Mar 16, 2022

@cataggar thank you for the comment and attention.

Changing from 'Resource' to 'SubResourceReadOnly' will remove 'name', 'type', 'location', and 'tags' property.
However, a VMExtension object has those properties inherently from the backend.
Utilizing PowerShell cmdlet, if I do:

PS C:\Users\thchan> Get-AzVMExtension -ResourceGroupName theo_test -VMName vmsdk

ResourceGroupName       : theo_test
VMName                  : vmsdk
Name                    : Microsoft.Azure.Geneva.GenevaMonitoring
Location                : eastus2
Etag                    : null
Publisher               : Microsoft.Azure.Geneva
ExtensionType           : GenevaMonitoring
TypeHandlerVersion      : 2.0
Id                      : /subscriptions/e37510d7-33b6-4676-886f-ee75bcc01871/resourceGroups/theo_test/providers/Micros
                          oft.Compute/virtualMachines/VmSdk/extensions/Microsoft.Azure.Geneva.GenevaMonitoring
PublicSettings          : {}
ProtectedSettings       :
ProvisioningState       : Succeeded
Statuses                :
SubStatuses             :
AutoUpgradeMinorVersion : True
ForceUpdateTag          :
EnableAutomaticUpgrade  : True

ResourceGroupName       : theo_test
VMName                  : vmsdk
Name                    : Microsoft.Azure.Security.AntimalwareSignature.AntimalwareConfiguration
Location                : eastus2
Etag                    : null
Publisher               : Microsoft.Azure.Security.AntimalwareSignature
ExtensionType           : AntimalwareConfiguration
TypeHandlerVersion      : 2.0
Id                      : /subscriptions/e37510d7-33b6-4676-886f-ee75bcc01871/resourceGroups/theo_test/providers/Micros
                          oft.Compute/virtualMachines/VmSdk/extensions/Microsoft.Azure.Security.AntimalwareSignature.An
                          timalwareConfiguration
PublicSettings          : {}
ProtectedSettings       :
ProvisioningState       : Succeeded
Statuses                :
SubStatuses             :
AutoUpgradeMinorVersion : True
ForceUpdateTag          :
EnableAutomaticUpgrade  : True

You can see them present.

I am exploring another solution: creating a new object called "ResouceWithOptionalLocation" which has the same properties but without having location tagged as 'required'. You can see my change 'here' I am working with the Swagger team for this changes impact on SDKs (track 2 and 1). I will keep you updated.

@cataggar
Copy link
Member

I see. That makes sense. Thanks @grizzlytheodore.

@grizzlytheodore
Copy link
Member

PR made to resolve: #18487
it will be released with CRP 2022-03-01 version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. Compute - Extensions Compute - VM Service Attention Workflow: This issue is responsible by Azure service team.
Projects
None yet
Development

No branches or pull requests

9 participants