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

Added v3.0 of the Azure REST API Guidelines #191

Merged
merged 13 commits into from
Apr 9, 2020
Merged

Added v3.0 of the Azure REST API Guidelines #191

merged 13 commits into from
Apr 9, 2020

Conversation

adrianhall
Copy link
Contributor

Per email discussion, this moves the current version of the Azure REST API Guidelines (converted from Word to Markdown, with minor spelling mistake corrections and formatting adjustments) into the api-guidelines repository.

@johngossman
Copy link
Contributor

We need to figure out what to do about the link to the Azure Stack versioning guidelines. I'd delete that section for now.

@johngossman
Copy link
Contributor

There is a link to an internal sharepoint site that is used for communicating deprecation. I'd suggest we just tell people to contact the API review board, which we do elsewhere in the deprecation guidance. That whole section needs to be read very closely that we aren't leaking internal addresses or processes that externals can't use, but I think it is nice to show customers what we do, and also for partners who are operating services, it is relevant...assuming they have a way to reach in to our comms and groups.

azure/Guidelines.md Outdated Show resolved Hide resolved

Teams building ARM RPs MUST follow the additional guidance in the ARM RPC and related documents. These documents can be found here.

* [Azure Resource Manager Documents][2] (Internal only)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these are public on Github now

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll dig - if anyone happens to know where, let me know. I'd rather include public docs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

azure/Guidelines.md Outdated Show resolved Hide resolved

The decision next **MUST** be communicated to customers and partners. The Service Notices site gives instructions for communicating this and other service changes. The Playbook contains a link to a submission form to request e-mail be sent.

https://microsoft.sharepoint.com/teams/azurecomms/SitePages/Email-Request.aspx
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's change this to "contact the Azure API review board" on how to start communicate externally for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that azureapirbcore or some other review board?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets just say Azure API review board and make it harder for spammers to find the alias...


<dl>
<dd>2020-Mar-31 v3.0</dd>
<dt>Version 3.0 of the Azure REST API Guidelines was copied from Sharepoint
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This process story seems pretty irrelevant to a public doc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've replaced it with a table and notes. I've made this release notes a little "obscure"


## Introduction

The Azure REST API guidelines are an extension of the Microsoft-wide [OneAPI guidelines][1] (which historically drew heavily from an earlier version of the Azure REST API guidelines). Readers of this document are assumed to be also reading the [OneAPI guidelines][1] and be familiar with them. Azure guidance is a superset of OneAPI guidelines and services should follow them *except* where this document outlines specific differences or exceptions to those guidelines. This document does contain additional Azure-specific guidance and additional details.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't use the term OneAPI in public docs. Need to decide what short name we're going to use for the core doc.

azure/Guidelines.md Outdated Show resolved Hide resolved

## Introduction

The Azure REST API guidelines are an extension of the Microsoft-wide [OneAPI guidelines][1] (which historically drew heavily from an earlier version of the Azure REST API guidelines). Readers of this document are assumed to be also reading the [OneAPI guidelines][1] and be familiar with them. Azure guidance is a superset of OneAPI guidelines and services should follow them *except* where this document outlines specific differences or exceptions to those guidelines. This document does contain additional Azure-specific guidance and additional details.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not OneAPI


## Introduction

The Azure REST API guidelines are an extension of the Microsoft-wide [OneAPI guidelines][1] (which historically drew heavily from an earlier version of the Azure REST API guidelines). Readers of this document are assumed to be also reading the [OneAPI guidelines][1] and be familiar with them. Azure guidance is a superset of OneAPI guidelines and services should follow them *except* where this document outlines specific differences or exceptions to those guidelines. This document does contain additional Azure-specific guidance and additional details.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not OneAPI

4. In addition to the functionality described here, services may support HTTP `OPTIONS` requests for other purposes such as further discovery, CORS, etc.
5. Services may allow unauthenticated HTTP `OPTIONS` requests. When doing so authors need to consider whether HTTP `OPTIONS` requests against non-existing resources result in 404s and whether that is leaking sensitive information. Certain scenarios, such as support for CORS pre-flight requests, require allowing unauthenticated HTTP `OPTIONS` requests.
6. For services that do rolling updates where there is a point in time where some front-ends are ahead of others version-wise, all front-ends should report the previous version as the latest version until the rolling update covers all instances and only then switch over to reporting the new latest version. This ensures that clients will not detect a version and then get load-balanced into a front-end that does not support it yet.
7. If using OData and addressing an expanded resource, the HTTP `OPTIONS` request should return the group versions that are supported across the expanded set.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SHOUL but really, only a shoudl here, I woudl have expected a MUST.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another topic to bring up :-)


Service teams **MUST** contact the Azure API review board before communicating the deprecation externally to customers and partners (which starts the 12 month clock).

Refer to the Azure deprecation policy for more details.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

link

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ref: @johngossman Is there a link I can link to for internal teams here?

azure/Guidelines.md Show resolved Hide resolved
azure/Guidelines.md Show resolved Hide resolved
azure/Guidelines.md Outdated Show resolved Hide resolved
azure/Guidelines.md Outdated Show resolved Hide resolved
azure/Guidelines.md Outdated Show resolved Hide resolved
@@ -87,11 +87,11 @@ http(s):https://<tenant-id>-<service-defined-root>.<service>.azure.net

All Azure APIs **MUST** use explicit versioning. It's critical that clients can count on services to be stable over time, and it's critical that Azure services can add features and make changes.

The OneAPI guidelines offer a couple of different options on how to specify an API version and guidance on what constitutes a breaking change. This section of the Azure API guidelines describes which options are required of Azure services as well as some guidance about deprecation policy. There is also a section about additional versioning practices necessary to support Azure Stack and Azure compatibility.
The Microsoft API guidelines offer a couple of different options on how to specify an API version and guidance on what constitutes a breaking change. This section of the Azure API guidelines describes which options are required of Azure services as well as some guidance about deprecation policy. There is also a section about additional versioning practices necessary to support Azure Stack and Azure compatibility.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docs is called the Microsoft REST API guidelines - we have lots of other API guidelines.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grammar nits: Consider removing "a couple of" and "as some". They're just extra words that don't add anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't like the way this was worded - so I word-smithed. I think it displays intent better now.

Copy link
Contributor

@cleemullins cleemullins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really great. Awesome to see this moving forward.

azure/Guidelines.md Outdated Show resolved Hide resolved
azure/Guidelines.md Outdated Show resolved Hide resolved
azure/Guidelines.md Outdated Show resolved Hide resolved
azure/Guidelines.md Outdated Show resolved Hide resolved
@@ -87,11 +87,11 @@ http(s):https://<tenant-id>-<service-defined-root>.<service>.azure.net

All Azure APIs **MUST** use explicit versioning. It's critical that clients can count on services to be stable over time, and it's critical that Azure services can add features and make changes.

The OneAPI guidelines offer a couple of different options on how to specify an API version and guidance on what constitutes a breaking change. This section of the Azure API guidelines describes which options are required of Azure services as well as some guidance about deprecation policy. There is also a section about additional versioning practices necessary to support Azure Stack and Azure compatibility.
The Microsoft API guidelines offer a couple of different options on how to specify an API version and guidance on what constitutes a breaking change. This section of the Azure API guidelines describes which options are required of Azure services as well as some guidance about deprecation policy. There is also a section about additional versioning practices necessary to support Azure Stack and Azure compatibility.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grammar nits: Consider removing "a couple of" and "as some". They're just extra words that don't add anything.

azure/Guidelines.md Outdated Show resolved Hide resolved
azure/Guidelines.md Outdated Show resolved Hide resolved

#### New property added to response

If a new property/field is added to the response of an API, the GET-PUT pipeline will be broken. Consider the case where a customer updates the value of a new property "A" from the Azure portal. Another customer does a GET of this resource using the SDK. The SDK will ignore the property since it does not understand it. From the SDK, the customer does a PUT using the model that was returned from the GET. This will overwrite the change made by the first customer from the portal.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be increasingly at-odds with standard web development practices. I'm not sure how long Azure will be able to hold the line on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's certainly a topic that can be visited during the update meetings that are planned.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Graph SDKs are able to avoid this problem because unknown properties retrieved from the server are round tripped. This follows the guidelines defined by the Atom publishing protocol. It is worth mentioning that Microsoft Graph does not make extensive use of PUT.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And please note that we've had instances where a service would reject a call with a read-only property that a client didn't know, but still tried to round-trip. The requirement for client behavior has to be in lock step with how a service is to treat unknown or read-only properties.


API version discovery is needed when either a given hosted service may expose a different API version to different clients (e.g. latest API version only available in certain regions or to certain tenants) or the service itself may exist in different instances (e.g. a service that may be run on Azure or hosted on-premises). In both of those cases clients may get ahead of services in the API version they use. In might also be possible for a client version to ship ahead of its corresponding service update, leading to the same situation. Lastly, version discovery is useful for clients that want to warn operators that an API they depend on may expire soon.

Azure services **SHOULD** support API version discovery. If they support it:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a random point, I really like this section and the features it describes. Are there libraries to help with this, or is it all an exercise to the reader? This seems like something that deserves a speclet (or even a DRAFT RFC!) on it's own.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote a javascript library for this a couple of years ago, but never got around to putting it into practice. The reality is that I locked the code down to a specific API and then depended on the contract not changing.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cleemullins, are you asking about libraries to help the service implement the behavior, or for guidance/libraries that help clients dynamically discover and light up features based on version availability?

azure/Guidelines.md Outdated Show resolved Hide resolved
azure/Guidelines.md Outdated Show resolved Hide resolved

Even though we recommend clients ignore new fields, there are many libraries and clients that are strict. Therefore, Azure services **MUST** update the version number of their API even when adding optional fields. In fact, servers should be as strict as possible. Ignoring a field can result in the API accepting content that containered a typo or an element at the wrong level of nesting. If this missing field changes the semantics (for example, we have seen cases where security settings were misplaced and ignored, leaving the resources more exposed than intended) this can be a huge and hard to discover error.

At a high level, changes to the contract of an API constitute a breaking change. Changes that impact backwards compatibility of an API is also considered a breaking change. Teams MAY define backwards compatibility as their business needs require. For example, Azure defines the addition of a new JSON field in a response to be not backwards compatible. Anything that would violate the Principle of Least Astonishment is considered a breaking change in Azure. Below are some concrete examples of what constitutes a breaking change. In the below breaking change scenarios, the API version must be changed.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a central Azure-wide group should define, review, and approve the compatibility bar. Incompatibilities hurt the Azure brand (i.e. all services), but benefit (and that is debatable too) only/mainly the individual service doing the break.


#### Existing property is removed

If a property called `foo` that was present in v1 of the API needs to be removed, it must be done in a newer API version.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish this section started with a strong statement describing why compatibility is important to gaining trust with enterprise customers and how we will be striving for 100% compact. Right now, it reads like a prescription for trivial breaking changes, like removing a "foo" field. If you want to use an example of a breaking change, I would suggest examples related to closing a security hole, privacy issue, or geo-political issue, which by the way are the only API breaking changes we should consider. These changes improve trust we build with customers, and so they offset the loss of trust caused by breaks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, We have such guidance currently in another doc, and want to merge that either here or up one level in the Microsoft guidelines in a future PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In reading this the first time, I agreed - we need a more meta "what will be considered for a breaking change" and then anything else is automatically not a breaking change. I think that wording can be a follow-on after an API review board meeting.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I believe this document was originally written specifically for resource providers/RPC. Which both means that it was more specific than general guidance (both based on specific issues that we wanted to prevent before a team even came to the board as well as things that are extra problematic for resource providers). In other words, it was a collection of case law rather than a holistic document.

azure/Guidelines.md Outdated Show resolved Hide resolved
azure/Guidelines.md Outdated Show resolved Hide resolved
azure/Guidelines.md Outdated Show resolved Hide resolved
azure/Guidelines.md Outdated Show resolved Hide resolved
azure/Guidelines.md Outdated Show resolved Hide resolved

#### Bug fixes to existing API

Bug fixes to existing API which don’t fall into one of the above categories of breaking changes as described above are fine.
Copy link
Contributor

@johanste johanste Apr 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implies that the list of breaking changes above is exhaustive. It is not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want to completely restructure the breaking change section as a first order of business .


#### Adding new APIs to an existing service

When a new resource types is added, it does not require API version to be updated for existing types.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is ARM/RPC specific (based on the use of the term "resource type")

Content-Length: 0
```

Clients that use version discovery are expected to cache version information. Since there’s a year of lead time after an API version shows in the `api-deprecated-versions` before it’s removed, checking once a week should provide sufficient lead time to client authors or operators. In the rare case where a server rolls back a version that clients are already using, the service will reject requests because they are ahead of the latest version supported. Whenever a client sees a `version-too-new` error, it should re-execute its version discovery procedure.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it still a minimum of one year?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd love to get clarification on this time frame - it comes up often.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now, I don't think the guidance of one year has officially changed. @johngossman - do you have any insight here?


Azure does not have a single SLA for how long we will support all services. However, we have published expectations such as [the Azure Cloud Lifecycle FAQ][6]. The most relevant section of the document:

> Azure Cloud Services will support no fewer than the latest two SDK versions for deploying new Cloud Services. Microsoft will provide notification 12 months before retiring a SDK in order to smooth the transition to a supported version.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is very relevant anymore (it refers to the RDFE cloud services and runtimes)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated it to the appropriate paragraph.

azure/Guidelines.md Outdated Show resolved Hide resolved
azure/Guidelines.md Outdated Show resolved Hide resolved
azure/Guidelines.md Outdated Show resolved Hide resolved
azure/Guidelines.md Outdated Show resolved Hide resolved
azure/Guidelines.md Outdated Show resolved Hide resolved
@annelo-msft
Copy link
Member

The structure of this document is possibly something to address later, but as a reader of these guidelines, I'd love it if we could more clearly draw parallels back to the MS REST API Guidelines -- possibly with common formatting to make it easy to identify visually in the document. It would be great if we could say in each section related to the MS Guidelines -- (1) this is the link to the section of the guidelines we're referring to (2) this is a correction, or a clarification of which option Azure uses, etc. In addition, it might be nice to group areas that aren't directly related to the MS Guidelines (it feels like the OpenAPI requirement is this?). Addressing this in an early stage might also help frame future discussions as well.

@garethj-msft garethj-msft merged commit 6084b10 into microsoft:vNext Apr 9, 2020
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

Successfully merging this pull request may close these issues.

8 participants