IMDS Swagger: Adding Swagger spec for 2019-08-15 version by jianyunt · Pull Request #6846 · Azure/azure-rest-api-specs

jianyunt · 2019-08-02T01:24:55Z

Latest improvements:

MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.

Contribution checklist:

I have reviewed the documentation for the workflow.
Validation tools were run on swagger spec(s) and have all been fixed in this PR.
The OpenAPI Hub was used for checking validation status and next steps.

ARM API Review Checklist

Service team MUST add the "WaitForARMFeedback" label if the management plane API changes fall into one of the below categories.
adding/removing APIs.
adding/removing properties.
adding/removing API-version.
adding a new service in Azure.

Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs.

If you are blocked on ARM review and want to get the PR merged urgently, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.
Please follow the link to find more details on API review process.

openapi-sdkautomation · 2019-08-02T01:24:58Z

In Testing, Please Ignore

[Logs] (Generated from f0938f7, Iteration 20)

Python: test-repo-billy/azure-sdk-for-python [Logs] [Diff]

No packages generated.

AutorestCI · 2019-08-02T01:25:32Z

Automation for azure-sdk-for-go

Nothing to generate for azure-sdk-for-go

AutorestCI · 2019-08-02T01:26:03Z

Automation for azure-sdk-for-python

Nothing to generate for azure-sdk-for-python

azuresdkci · 2019-08-02T02:01:52Z

Can one of the admins verify this patch?

PhoenixHe-NV · 2019-08-02T09:17:02Z

@OpenAPIBot sdkautomation rebuild

jianyunt · 2019-08-02T18:11:12Z

Hi @lirenhe and other admins, all checks are passed now. Please review the changes and approve and merge if possible. Thanks!

\cc @rifrankl @rikotcho @KumariSupriya @harijayms @edyoung

# Conflicts: # specification/imds/data-plane/readme.md

jianyunt · 2019-08-13T16:02:38Z

Hi @lirenhe and other admins, I have integrated the changes from IMDS 2019-08-01 and all checks are passed. Please let us know if anything we need to do to get the PR approved and merged?

@rifrankl @rikotcho @KumariSupriya @harijayms @edyoung

ryansbenson

Data plane APIs aren't reviewed by ARM. Removing label.

johanste · 2019-08-14T02:26:57Z

specification/imds/data-plane/Microsoft.InstanceMetadataService/stable/2019-08-15/imds.json

+                        "required": false
+                    },
+                    {
+                        "name": "mi_res_id",


Changing the name of the parameter is a breaking change (it was msi_res_id in the previous version). Why is this change necessary?

The name change was based on code review feedback from the Identity team. But I leave it out for now I changed back to msi_res_id.

What does the API actually expect? Was the description incorrect earlier and this was a correction, or was this a planned change to the REST API?

I am not 100% sure if imds is responding mi_res_id today. So I'd rather leave this particular change to the Identity team.

This needs to be sorted out before we can merge the PR. The description has to be correct w. regards to the actual REST API.

@johanste , do you mean whether msi_res_id or mi_res_id should be sorted it now? I looked file history, it has been msi_res_id. So no change for this version.

@johanste I looked identity team's source code, there are no changes in the history about msi_res_id. Also public docs are about msi_res_id.
This version is particularly for our hybrid scenario. No impact to the existing Azure IMDS.
If you have concerns, I can follow-up with the Metadata and Identity teams about it. However to fix the existing spec issues(if any), can we make a separate PR to fix the existing api versions all together. Thoughts?

Assuming that the name in the swagger is what the REST API actually expects/uses, then we are good. From you explanation, that seems to be the case. What we should not do is check in a swagger that is incorrect and then later fix it.

If the service team intends to change the name in the REST API in the future, please make sure that they consult the azure api review board to determine if the change is warranted.

Agreed. teams should consult with the board first for any breaking changes. I will follow-up with the Metadata and Identity team to see if there are any inconsistencies between code and the spec.
For this particular PR, do you have any other concerns? If not, could you please approve it? thanks!

specification/imds/data-plane/Microsoft.InstanceMetadataService/stable/2019-08-15/imds.json

PhoenixHe-NV

Why we only have python sdk to generate in swagger-to-sdk section? Do you want to add other sdks?

jianyunt · 2019-08-14T16:53:24Z

@NullMDR I am not sure if I got your point. And I see the build seems to cover sdk node, sdk java, go, python, etc. already.

PhoenixHe-NV · 2019-08-15T03:25:51Z

@jianyunt I mean the section here https://github.com/Azure/azure-rest-api-specs/pull/6846/files#diff-1f6ca4d24ed36009a22cccc573434122R223

We will move to new automation system in a near furture and you need that section to contain configuration for other languages to generate sdk in the new system.

jianyunt · 2019-08-15T17:06:17Z

@NullMDR what specific file do I need to change and how in order to adopt the new system you mentioned to cover other languages? it should be happening now or later?

johanste · 2019-08-15T17:35:40Z

No actual API changes. No need for the Azure API review board to review.

PhoenixHe-NV · 2019-08-16T02:39:41Z

@jianyunt https://github.com/Azure/azure-rest-api-specs/blob/f0938f745e66b378b3874224a929a56ff88cf5e9/specification/imds/data-plane/readme.md#swagger-to-sdk

You'd better update it before we switch to new sdk generation system. We'll upgrade to new system a few month later.

Updated spec version for the hybrid RP support

b2a45f1

azuresdkci assigned lirenhe Aug 2, 2019

azuresdkci requested a review from lirenhe August 2, 2019 01:58

jianyunt changed the title ~~Updated imds spec version for the hybrid RP support~~ Swagger:Updated imds spec version for the hybrid RP support Aug 2, 2019

jianyunt added 4 commits August 2, 2019 09:33

fixed model validation error

d113045

Fix model validation error

5aac108

updated GetIdentityToken sample to fix the model validation error

7cad7c9

fixed Avocado error

1e01c2d

jianyunt changed the title ~~Swagger:Updated imds spec version for the hybrid RP support~~ Swagger: Adding Swagger spec for 2019-08-15 version Aug 2, 2019

updated sample

89edd88

lirenhe added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Aug 2, 2019

jianyunt added 5 commits August 12, 2019 13:12

Merge remote-tracking branch 'upstream/master'

fddc66a

# Conflicts: # specification/imds/data-plane/readme.md

Integrated changes made in 2019-08-01

b874607

fix alignment

2eef038

fixed indentation

7ad56a7

fix alignment

84bd981

jianyunt changed the title ~~Swagger: Adding Swagger spec for 2019-08-15 version~~ IMDS Swagger: Adding Swagger spec for 2019-08-15 version Aug 13, 2019

ryansbenson removed the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Aug 13, 2019

ryansbenson reviewed Aug 13, 2019

View reviewed changes

lirenhe added the APIStewardshipBoard-ReviewRequested This should be reviewed by the Azure API Stewardship team in partnership with the service team. label Aug 14, 2019

lirenhe approved these changes Aug 14, 2019

View reviewed changes

johanste reviewed Aug 14, 2019

View reviewed changes

specification/imds/data-plane/Microsoft.InstanceMetadataService/stable/2019-08-15/imds.json Outdated Show resolved Hide resolved

PhoenixHe-NV reviewed Aug 14, 2019

View reviewed changes

address CR feedback

5a19bff

jianyunt added 3 commits August 14, 2019 11:52

Moved in headers to the default section

6795e3c

fixed modelvalidation error

a37e359

update

f0938f7

johanste removed the APIStewardshipBoard-ReviewRequested This should be reviewed by the Azure API Stewardship team in partnership with the service team. label Aug 15, 2019

PhoenixHe-NV mentioned this pull request Aug 16, 2019

[Mirror] IMDS Swagger: Adding Swagger spec for 2019-08-15 version test-repo-tih/azure-rest-api-specs#135

Closed

yungezz merged commit a0cd13d into Azure:master Aug 17, 2019

Conversation

jianyunt commented Aug 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Latest improvements:

Contribution checklist:

ARM API Review Checklist

Uh oh!

openapi-sdkautomation bot commented Aug 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

In Testing, Please Ignore

[Logs] (Generated from f0938f7, Iteration 20)

Uh oh!

AutorestCI commented Aug 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-go

Uh oh!

AutorestCI commented Aug 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-python

Uh oh!

azuresdkci commented Aug 2, 2019

Uh oh!

PhoenixHe-NV commented Aug 2, 2019

Uh oh!

jianyunt commented Aug 2, 2019

Uh oh!

jianyunt commented Aug 13, 2019

Uh oh!

ryansbenson left a comment

Choose a reason for hiding this comment

Uh oh!

johanste Aug 14, 2019

Choose a reason for hiding this comment

Uh oh!

jianyunt Aug 14, 2019

Choose a reason for hiding this comment

Uh oh!

johanste Aug 14, 2019

Choose a reason for hiding this comment

Uh oh!

jianyunt Aug 14, 2019

Choose a reason for hiding this comment

Uh oh!

johanste Aug 15, 2019

Choose a reason for hiding this comment

Uh oh!

jianyunt Aug 15, 2019

Choose a reason for hiding this comment

Uh oh!

jianyunt Aug 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

johanste Aug 15, 2019

Choose a reason for hiding this comment

Uh oh!

jianyunt Aug 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

PhoenixHe-NV left a comment

Choose a reason for hiding this comment

Uh oh!

jianyunt commented Aug 14, 2019

Uh oh!

PhoenixHe-NV commented Aug 15, 2019

Uh oh!

jianyunt commented Aug 15, 2019

Uh oh!

johanste commented Aug 15, 2019

Uh oh!

PhoenixHe-NV commented Aug 16, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

jianyunt commented Aug 2, 2019 •

edited

Loading

openapi-sdkautomation bot commented Aug 2, 2019 •

edited

Loading

AutorestCI commented Aug 2, 2019 •

edited

Loading

AutorestCI commented Aug 2, 2019 •

edited

Loading

jianyunt Aug 15, 2019 •

edited

Loading

jianyunt Aug 15, 2019 •

edited

Loading