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

Ambiguous Permissions on Properties Complicate 75+ Create-OR-Update Operations #275

Closed
selvasingh opened this issue Apr 28, 2016 · 6 comments

Comments

@selvasingh
Copy link
Contributor

selvasingh commented Apr 28, 2016

Ambiguous Permissions on Properties Complicate 75+ Create-OR-Update Operations

There are 75+ Create-OR-Update operations (across Azure Swagger specs) with ambiguous, undocumented permissions on properties (or parameters). If permissions on properties were unambiguously represented in Swagger specs then a generated client library could offer simpler programming experiences and improved predictability.

Let us look at one example.

VirtualMachines.createOrUpdate() - one of the parameters is VirtualMachine. Here is a breakdown of permissions of a subset of properties of VirtualMachine:

Property Permission
id read (4)
storageProfile::imageReference read | initialize (6)
storageProfile::osDisk::createOption read | initialize (6)
storageProfile::osDisk::vhd::uri read | initialize (6)
availabilitySet read | initialize (6)
hardwareProfile read | initialize | update (7)
storageProfile::dataDisks read | initialize | update (7)
storageProfile::osDisk::caching read | initialize | update (7)
networkProfile read | initialize | update (7)
tags read | initialize | update (7)

Today, because of this ambiguous situation, when a developer calls the VirtualMachines.createOrUpdate() operation, the only available option is trial and error to find out what combinations of parameters work.

The good news is that it is possible to capture these permissions.

Swagger 2.0 says, "Declares the property as "read only". This means that it MAY be sent as part of a response but MUST NOT be sent as part of the request. Properties marked as readOnly being true SHOULD NOT be in the required list of the defined schema. Default value is false " Some of Azure Swagger specs use this property.

readOnly can only represent read permissions but CANNOT represent read | initialize (6) OR read | initialize | update (7). A new Swagger extension will solve the problem -

Field Name Type Description
mutability integer Represents the permission of the property:
-read (4)
-initialize (1)
-read | initialize (6)
-read | initialize | update (7)
Default is read | initialize | update (7)

Capturing permissions on properties will simplify programming experiences and improve predictability across languages - C#, Java, Node, Python, Ruby, PHP, Go - and tools - PowerShell and cross-platform CLI.

Today, regardless of the ambiguous situation, hand-written PowerShell commandlets and cross-platform CLI commands absorb the complexity and simplify it for developers - there are two distinct paths for Create and Update:

PowerShell: Create a virtual machine that uses VirtualMachines.createOrUpdate()

PS C:\> New-AzureRmVM -ResourceGroupName $ResourceGroupName -Location $Location -VM $VirtualMachine

PowerShell: Update a virtual machine that uses VirtualMachines.createOrUpdate()

PS C:\> Update-AzureRmVM -ResourceGroupName "ResourceGroup11" -Name "VirtualMachine07" -VM $VirtualMachine

If permissions on properties were unambiguously represented in Swagger specs then a generated client library could offer simpler programming experiences and improved predictability.

Reference - today, there are 75+ such Create-OR-Update operations with such ambiguous permissions on properties

Authorization

Compute

Data Lake Store Account

DNS

Intune

Logic

Network

Notification Hubs

Redis

Remote App

ARM Resources - Locks

ARM Resources - Account

Scheduler

Search

SQL

App Service - Web App

@selvasingh selvasingh changed the title Ambiguous Permissions on Properties Complicate Create-OR-Update Operations Ambiguous Permissions on Properties Complicate 75+ Create-OR-Update Operations Apr 28, 2016
@devigned
Copy link
Member

Why do we need read | initialize? Shouldn't that just be a readonly property? If they can't be updated in the API, why should we provide any way to initialize?

@Azure/adx-autorest-contributors thoughts?

@selvasingh
Copy link
Contributor Author

Please see a partial breakdown of permissions of a subset of properties of virtual machine where some of the parameters are part of a create operation, then it is read only

@devigned
Copy link
Member

@selvasingh the way I interpret that is that the constructor should require the readonly | initialize properties and not provide a setter for those properties. Anything that is readonly | initialize | update should be optional and provide a setter. Is that an incorrect interpretation?

@devigned
Copy link
Member

devigned commented May 9, 2016

OAI/OpenAPI-Specification#425 <-- perhaps, we should use composition as described here.

@selvasingh
Copy link
Contributor Author

@devigned
Copy link
Member

I'm going to close this issue and track it via #307.

blueww pushed a commit to blueww/azure-rest-api-specs that referenced this issue Dec 8, 2017
prepare new version pricing plan api for ARM team to review
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

No branches or pull requests

2 participants