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

AAD to Microsoft Entra updates #26725

Merged
merged 24 commits into from
Dec 12, 2023
Merged

Conversation

stevemunk
Copy link
Member

There are no new or modified APIs in this PR, only changes to the documentation that are related to the AAD to Entra rebranding.

@stevemunk stevemunk requested a review from a team as a code owner November 16, 2023 00:31
@stevemunk stevemunk requested review from tg-msft and weidongxu-microsoft and removed request for a team November 16, 2023 00:31
Copy link

Automatic PR validation started. This comment will be populated with next steps to merge this PR once validation is completed. Please wait ⌛.

Copy link

Swagger pipeline can not start as the pull request is not merged or mergeable - most likely it has merge conflicts.

1 similar comment
Copy link

Swagger pipeline can not start as the pull request is not merged or mergeable - most likely it has merge conflicts.

Copy link

Swagger pipeline started successfully. If there is ApiView generated, it will be updated in this comment.

Copy link

openapi-pipeline-app bot commented Nov 16, 2023

Next Steps to Merge

✔️ All automated merging requirements have been met! Refer to step 4 in the PR workflow diagram (even if your PR is for data plane, not ARM).

Copy link

openapi-pipeline-app bot commented Nov 16, 2023

Swagger Validation Report

️️✔️BreakingChange succeeded [Detail] [Expand]
There are no breaking changes.
compared swaggers (via Oad v0.10.4)] new version base version
common.json 1.0(2314dca) 1.0(main)
common.json 2022-09-01-preview(2314dca) 2022-09-01-preview(main)
spatial.json 1.0(2314dca) 1.0(main)
alias.json 2.0(2314dca) 2.0(main)
data.json 2.0(2314dca) 2.0(main)
dataset.json 2.0(2314dca) 2.0(main)
dwgconversion.json 2.0(2314dca) 2.0(main)
featurestate.json 2.0(2314dca) 2.0(main)
tileset.json 2.0(2314dca) 2.0(main)
wfs.json 2.0(2314dca) 2.0(main)
dataset.json 2022-09-01-preview(2314dca) 2022-09-01-preview(main)
mapconfiguration.json 2022-09-01-preview(2314dca) 2022-09-01-preview(main)
routeset.json 2022-09-01-preview(2314dca) 2022-09-01-preview(main)
style.json 2022-09-01-preview(2314dca) 2022-09-01-preview(main)
tileset.json 2022-09-01-preview(2314dca) 2022-09-01-preview(main)
wayfind.json 2022-09-01-preview(2314dca) 2022-09-01-preview(main)
dataregistry.json 2023-06-01(2314dca) 2023-06-01(main)
geolocation.json 1.0(2314dca) 1.0(main)
alias.json 2.0(2314dca) 2.0(main)
elevation.json 1.0(2314dca) 1.0(main)
data.json 1.0(2314dca) 1.0(main)
data.json 2.0(2314dca) 2.0(main)
dataset.json 2.0(2314dca) 2.0(main)
dwgconversion.json 2.0(2314dca) 2.0(main)
featurestate.json 2.0(2314dca) 2.0(main)
feedback.json 1.0(2314dca) 1.0(main)
geolocation.json 1.0(2314dca) 1.0(main)
render.json 1.0(2314dca) 1.0(main)
render.json 2.0(2314dca) 2.0(main)
route.json 1.0(2314dca) 1.0(main)
search.json 1.0(2314dca) 1.0(main)
spatial.json 1.0(2314dca) 1.0(main)
tileset.json 2.0(2314dca) 2.0(main)
timezone.json 1.0(2314dca) 1.0(main)
traffic.json 1.0(2314dca) 1.0(main)
wfs.json 2.0(2314dca) 2.0(main)
weather.json 1.0(2314dca) 1.0(main)
render.json 2022-08-01(2314dca) 2022-08-01(main)
route.json 1.0(2314dca) 1.0(main)
search.json 1.0(2314dca) 1.0(main)
spatial.json 2022-08-01(2314dca) 2022-08-01(main)
timezone.json 1.0(2314dca) 1.0(main)
traffic.json 1.0(2314dca) 1.0(main)
weather.json 1.1(2314dca) 1.1(main)
️️✔️Breaking Change(Cross-Version) succeeded [Detail] [Expand]
There are no breaking changes.
️️✔️CredScan succeeded [Detail] [Expand]
There is no credential detected.
️🔄LintDiff inProgress [Detail]
️⚠️Avocado: 1 Warnings warning [Detail]
Rule Message
⚠️ MULTIPLE_API_VERSION The default tag contains multiple API versions swaggers.
readme: specification/maps/data-plane/readme.md
tag: specification/maps/data-plane/readme.md#tag-package-stable-2023-07-01
️❌SwaggerAPIView: 0 Errors, 0 Warnings failed [Detail]
️️✔️TypeSpecAPIView succeeded [Detail] [Expand]
️️✔️ModelValidation succeeded [Detail] [Expand]
Validation passes for ModelValidation.
️️✔️SemanticValidation succeeded [Detail] [Expand]
Validation passes for SemanticValidation.
️❌PoliCheck: 1 Errors, 0 Warnings failed [Detail]
Rule Message
Click detail for error messages. Exception contact vsswagger@microsoft.com or https://aka.ms/swaggersupport.
️️✔️SpellCheck succeeded [Detail] [Expand]
Validation passes for SpellCheck.
️️✔️Lint(RPaaS) succeeded [Detail] [Expand]
Validation passes for Lint(RPaaS).
️️✔️PR Summary succeeded [Detail] [Expand]
Validation passes for Summary.
️️✔️Automated merging requirements met succeeded [Detail] [Expand]
Posted by Swagger Pipeline | How to fix these errors?

Copy link

openapi-pipeline-app bot commented Nov 16, 2023

Swagger Generation Artifacts

️️✔️ApiDocPreview succeeded [Detail] [Expand]
 Please click here to preview with your @microsoft account. 
️️✔️SDK Breaking Change Tracking succeeded [Detail] [Expand]

Breaking Changes Tracking

Posted by Swagger Pipeline | How to fix these errors?

Copy link

openapi-pipeline-app bot commented Nov 16, 2023

PR validation pipeline restarted successfully. If there is ApiView generated, it will be updated in this comment.

@stevemunk
Copy link
Member Author

@tg-msft
@weidongxu-microsoft
@tjprescott

Please advise on what I need to do to get this PR reviewed and merged. The Swagger ModelValidation errors have nothing to do with the changes that I've made, and they appear to be false negatives. There are two types of errors that I see:

1: RESPONSE_SCHEMA_NOT_IN_SPEC

Response statusCode '{0}' for operation '{1}' has response body provided in the example, however the response does not have a "schema" defined in the swagger spec.

In call cases I've looked at, a response is defined. for example:

         "responses": {
          "200": {
            "$ref": "../../../Common/preview/1.0/common.json#/responses/200AsyncV2"
          }	

If you look at what it is referencing in "../../../Common/preview/1.0/common.json#/responses/200AsyncV2":

     "200AsyncV2": {
      "description": "The operation is running or complete. If the operation was successful, use the Resource-Location header to obtain the path to the result.",
      "schema": {
        "$ref": "#/definitions/LongRunningOperationResult"
      }

You can look at what it is referencing in "#/definitions/LongRunningOperationResult" here

It appears that the response does have a "schema" defined in the swagger spec, if not please advise.

2: INVALID_TYPE

The first invalid type error is referring to the bounding box property which is defined as an array of numbers (doubles) see here. This is correct.

In the example, it shows this: "bbox": "-122, 47, -120, 46"

It appears that this is correct, if not please advise.

@@ -68,7 +86,6 @@ input-file:
- Microsoft.Maps/Timezone/preview/1.0/timezone.json
- Microsoft.Maps/Traffic/preview/1.0/traffic.json
- Microsoft.Maps/Weather/preview/1.0/weather.json
- Microsoft.Maps/WFS/preview/2.0/wfs.json
Copy link
Member

Choose a reason for hiding this comment

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

I believe this removal is why you're seeing the sudden errors for unreferenced json from readme. the first avocado error specifically. I think it's iffy reporting, but all stemming from the unreferenced wfs.json. I'm betting it's breaking oav's logic when resolving the swagger itself, which is why the examples are also throwing.

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 if you make changes to the readme like this, you're going to need to go through the ARM review process as described in the PR body. (the image with labeling instructions etc in the automatically created body of the PR).

@mikeharder
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@scbedd
Copy link
Member

scbedd commented Dec 8, 2023

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Copy link
Member

@tjprescott tjprescott left a comment

Choose a reason for hiding this comment

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

Approved in error and GH won't let me dismiss

@stevemunk
Copy link
Member Author

@tjprescott, @scbedd, @weidongxu-microsoft: 99.9% of all errors have been addressed, that leaves us with one error. It is a PoliCheck error:

image

Pretty sure these three letters are not used anywhere in this swagger file (or any other Azure Maps swagger files). I looked in SuccessfulWorldCopyrightRequest.json and couldn't find those three letters together except when part of 'Istituto' and 'constitute', and the following URL:

image

Any suggestions?

@tjprescott
Copy link
Member

@stevemunk the blocking issue wasn't the policheck failure--it was the need to review and approve the suppression statements you added.

@tjprescott tjprescott merged commit e8bd7af into Azure:main Dec 12, 2023
28 of 30 checks passed
arifibrahim4 pushed a commit that referenced this pull request Feb 26, 2024
* AAD to Microsoft Entra updates

* Added getitems to ./custom-words.txt as instructed in https://aka.ms/azsdk/ci-fix.

* Removed the extra space before 'Microsoft Entra ID'.

* Added two suppressions: 1-INVALID_TYPE 2-RESPONSE_SCHEMA_NOT_IN_SPEC.

* Changed API version in examples to address INVALID_REQUEST_PARAMETER errors.

* Changed API version in examples to address INVALID_REQUEST_PARAMETER errors.

* add suppressions for the errors under specification/maps/data-plane/Creator/ that shouldn't be popping up anymore

* add suppression for INVALID_TYPE in specification/maps/data-plane/Render/readme.md

* added suppressions

* removed duplicate section.

* reverting changes to these files. As near as I can determine, these are not customer facing and making the AAD to Microsoct Entra rebranding changes is resulting is hundreds of errors.

* LRO_RESPONSE_HEADER errors

* LRO_RESPONSE_HEADER errors

* addressing PoliCheck issues.

* addressing PoliCheck issues.

* addressing PoliCheck issues.

* addressing  LRO_RESPONSE_HEADER errors.

* addressing PoliCheck errors.

* addressing PoliCheck errors.

---------

Co-authored-by: Scott Beddall (from Dev Box) <scbedd@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants