Incorrect handling of custom images

[puhley]

Information

• Onefuzz version: 3.2.0
• OS: Azure Function App

Provide detailed reproduction steps (if any)

1. The documentation for using custom OS images within OneFuzz scale sets (https://github.com/microsoft/onefuzz/blob/main/docs/custom-images.md) says that ti accepts Azure Compute Gallery resource IDs, such as: /subscriptions/MYSUBSCRIPTION/resourceGroups/MYGROUP/providers/Microsoft.Compute/galleries/MYGALLERY/images/MYDEFINITION/versions/MYVERSION
2. When creating a VM scale set, the get_os function gets called to identify the operating system type of the image within create_vmss (onefuzzlib/azure/vmss.py - line 317). The get_os function of image.py in the onefuzzlib library (https://github.com/microsoft/onefuzz/blob/main/src/api-service/__app__/onefuzzlib/azure/image.py) attempts to fetch the image information using "client.images.get" (line 25). Unfortunately, that Azure SDK method fetches definitions from Azure Images and not Azure Compute Galleries.
3. The correct code for supporting Azure Compute Galleries would be:

name = client.gallery_images.get(parsed["resource_group"], parsed["name"], parsed["child_name_1"]).os_type.lower()

4. There is also a second issue in the same section of code. The existing code for retrieving the image from Azure Images tries to access the 'name' property of the 'os_type' which does not exist. This should instead be a call to '.lower()'. Both issues are described in further detail in the Actual Result section below.

Expected result

It does not appear to be possible to create a scale set from an Azure Image. Therefore, the code needs to be updated to correctly handle Compute Galleries. Assuming that the Azure Image approach within get_os is necessary for other workflows within OneFuzz, the os_type reference to the 'name' property should be replaced with a call to '.lower()'.

Actual result

The code throws a ResourceNotFound error because it is given an Azure Compute Gallery resource ID and it is expecting an Azure Images resource ID. The code needs to be updated to leverage client.gallery_images.get() instead in order to support Azure Compute Galleries (https://docs.microsoft.com/en-us/python/api/azure-mgmt-compute/azure.mgmt.compute.v2021_07_01.operations.galleryimagesoperations?view=azure-python#get-resource-group-name--gallery-name--gallery-image-name----kwargs-).

In addition, existing references to Azure Images fail due to trying to access a nonexistent property of the os_type. It appears that the existing code is expecting a resource ID for Azure Images, such as:

/subscriptions/MYSUBSCRIPTION/resourceGroups/MYGROUP/providers/Microsoft.Compute/images/MYIMAGE

However, if the above image resource ID from Azure Images is provided, then the code will still result in an error. In this case, the get_os code is able to find the Azure Image resource. Unfortunately, the code then tries to access a nonexistent property of that image when it executes line 27:

.storage_profile.os_disk.os_type.name

For Azure Images, the 'os_type' is a string / OperatingSystemsType Enum and it does not have a 'name' property (https://docs.microsoft.com/en-us/python/api/azure-mgmt-compute/azure.mgmt.compute.v2021_07_01.models.imageosdisk?view=azure-python). In this case, the code should be: .os_type.lower() similar to line 39.

Proposed solution

I do not know the code well enough to know if the existing get_os parsing behavior is necessary for the other use cases within the overall application. If it is necessary, then an if clause could be added to preserve backwards compatibility. The following code change for /onefuzzlib/azure/image.py (lines 23-30) includes the 'os_type' fix for Azure Images to address the nonexistent 'name' property issue, and it allows support for both Compute Galleries and Images reference IDs:

    if "resource_group" in parsed:
        if parsed["type"] == "galleries":
            try:
                name = client.gallery_images.get(
                    parsed["resource_group"], parsed["name"], parsed["child_name_1"]
                ).os_type.lower()
            except (ResourceNotFoundError, CloudError) as err:
                return Error(code=ErrorCode.INVALID_IMAGE, errors=[str(err)])
        else: 
            try:
                name = client.images.get(
                    parsed["resource_group"], parsed["name"]
                ).storage_profile.os_disk.os_type.lower()
            except (ResourceNotFoundError, CloudError) as err:
                return Error(code=ErrorCode.INVALID_IMAGE, errors=[str(err)])
    else:
           ...

Per the contributing guidelines, I am holding off on submitting a pull request until the issue gets reviewed.

[ranweiler]

Thanks for the detailed bug report, @puhley (and for reading the contribution guidelines!). We would be happy to review a PR addressing this.

[mgreisen]

Closing. PR #1450 addressed this issue.