Advanced ingress - ACA - Swagger API - DELETED - NEW PR - https://github.com/Azure/azure-rest-api-specs/pull/30862 by tdaroly · Pull Request #30660 · Azure/azure-rest-api-specs

tdaroly · 2024-09-20T06:08:23Z

NEW PR: #30862

Choose a PR Template

Switch to "Preview" on this description then select one of the choices below.

Click here to open a PR for a Data Plane API.

Click here to open a PR for a Control Plane (ARM) API.

openapi-pipeline-app · 2024-09-20T06:08:27Z

Next Steps to Merge

Next steps that must be taken to merge this PR:

❌ This PR is in purview of the ARM review (label: ARMReview). This PR must get ARMSignedOff label from an ARM reviewer.
This PR has ARMChangesRequested label. Please address or respond to feedback from the ARM API reviewer.
When you are ready to continue the ARM API review, please remove the ARMChangesRequested label.
Automation should then add WaitForARMFeedback label.
❗If you don't have permissions to remove the label, request write access per aka.ms/azsdk/access#request-access-to-rest-api-or-sdk-repositories.
For details of the ARM review, see aka.ms/azsdk/pr-arm-review
❌ The required check named Automated merging requirements met has failed. This is the final check that must pass. Refer to the check in the PR's 'Checks' tab for details on how to fix it and consult the aka.ms/ci-fix guide. In addition, refer to step 4 in the PR workflow diagram

openapi-pipeline-app · 2024-09-20T06:08:27Z

Generated ApiView

Language	Package Name	ApiView Link
JavaScript	@azure/arm-appcontainers	https://apiview.dev/Assemblies/Review/ba41cd9978b542efb0688a072b2319a1?revisionId=34499d98172e44dc954bd314c141ce61
Java	azure-resourcemanager-appcontainers	https://apiview.dev/Assemblies/Review/7da9ac1e2db64477ada2ba8bbb71169b?revisionId=1e94ee6e08a14a56bd61af6b1292f62e
Swagger	Microsoft.App	https://apiview.dev/Assemblies/Review/53999dbad23b45cba7812a00a713cab8?revisionId=2a879c210f4b4994ad33b4a4428bc900

razvanbadea-msft · 2024-09-20T13:42:35Z

Choose a PR Template description and add a description to this pr to have the Purpose of PR and due diligence sections added

razvanbadea-msft · 2024-09-20T13:46:51Z

...ication/app/resource-manager/Microsoft.App/preview/2024-08-02-preview/CommonDefinitions.json

+      "properties": {
+        "tags": {
+          "type": "object",
+          "additionalProperties": {


additionalProperties

ARM is trying to reduce instances of unschematized APIs to make it easier for consumers of those APIs to understand them and use them reliably, not to mention provide better support in the various SDKs.

Please make this an array or provide an explanation for why you need this.

this is the PATCH response. Why would this need to be an array ? Could you give me an example spec of using arrays to for PATCH request/response ?

@razvanbadea-msft can we resolve this conversation? this pattern is similar to all other tag props in the CommonDefintions spec

We dont need to add tags property as it is included with tracked property reference.
Refer

azure-rest-api-specs/specification/common-types/resource-management/v5/types.json

Line 61 in d779281

"tags": {

This does not need explicit declaration as it gets included with tracked resource definition

azure-rest-api-specs/specification/common-types/resource-management/v5/types.json

Line 61 in d779281

"tags": {

This is for PATCH response and i see other resources using this pattern.

Managed Cert

azure-rest-api-specs/specification/app/resource-manager/Microsoft.App/preview/2024-08-02-preview/CommonDefinitions.json

Line 1608 in 508abd0

"tags": {

Cert

azure-rest-api-specs/specification/app/resource-manager/Microsoft.App/preview/2024-08-02-preview/CommonDefinitions.json

Line 1534 in 508abd0

"tags": {

Can we keep this pattern ?

...ication/app/resource-manager/Microsoft.App/preview/2024-08-02-preview/CommonDefinitions.json

razvanbadea-msft · 2024-09-20T14:19:21Z

...ication/app/resource-manager/Microsoft.App/preview/2024-08-02-preview/CommonDefinitions.json

+        }
+      }
+    },
+    "HttpRouteMatch": {


HttpRouteMatch

can you have prefix and path and pathSeparatedPrefix set at once? In place of having this properties that could grow in number, an enum property like
matchType with [Prefix, CaseSensitivePath, Path, PathSeparatedPrefix] and a matchValue string. In this way if another criteria will be needed youii could only have to add a new value to the enum

No, some of these are mutually exclusive.

This matches the Envoy API we are exposing, we're trying not to diverge from that so that

it's easier to expose additional envoy APIs in the future

it's easier for customers to understand each field's behavior from ACA or envoy docs

it's easier to validate the format of each field

it's simpler usage. You'll normally only be setting one field. The enum would always require two fields, the type and the value

...ation/app/resource-manager/Microsoft.App/preview/2024-08-02-preview/ManagedEnvironments.json

razvanbadea-msft · 2024-09-20T14:26:19Z

Fix the Swagger PrettierCheck and Swagger ModelValidation checks

...ication/app/resource-manager/Microsoft.App/preview/2024-08-02-preview/CommonDefinitions.json

jijohn14 · 2024-10-04T05:45:30Z

...ication/app/resource-manager/Microsoft.App/preview/2024-08-02-preview/CommonDefinitions.json

+      "properties": {
+        "tags": {
+          "type": "object",
+          "additionalProperties": {


We dont need to add tags property as it is included with tracked property reference.
Refer

azure-rest-api-specs/specification/common-types/resource-management/v5/types.json

Line 61 in d779281

"tags": {

...ication/app/resource-manager/Microsoft.App/preview/2024-08-02-preview/CommonDefinitions.json

jijohn14 · 2024-10-04T05:48:33Z

...ication/app/resource-manager/Microsoft.App/preview/2024-08-02-preview/CommonDefinitions.json

+      "properties": {
+        "tags": {
+          "type": "object",
+          "additionalProperties": {


This does not need explicit declaration as it gets included with tracked resource definition

azure-rest-api-specs/specification/common-types/resource-management/v5/types.json

Line 61 in d779281

"tags": {

...ication/app/resource-manager/Microsoft.App/preview/2024-08-02-preview/CommonDefinitions.json

...ation/app/resource-manager/Microsoft.App/preview/2024-08-02-preview/ManagedEnvironments.json

openapi-pipeline-app · 2024-10-04T20:55:00Z

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

Taher Darolywala added 2 commits September 19, 2024 22:21

commit inital changes

81be463

add example jsons

07a0d62

tdaroly requested review from Juliehzl and jijohn14 as code owners September 20, 2024 06:08

azure-sdk mentioned this pull request Sep 20, 2024

[AutoPR sdk/resourcemanager/appcontainers/armappcontainers] Advanced ingress - ACA - Swagger API azure-sdk/azure-sdk-for-go#11516

Closed

AzureRestAPISpecReview added ARMReview resource-manager WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Sep 20, 2024

azure-sdk added the BreakingChange-Python-Sdk label Sep 20, 2024

Taher Darolywala added 2 commits September 20, 2024 00:53

add patch support, fix camelcasing

78b8f35

fix linting, long running opp, pretty menv.json

3ccf073

razvanbadea-msft reviewed Sep 20, 2024

View reviewed changes

...ication/app/resource-manager/Microsoft.App/preview/2024-08-02-preview/CommonDefinitions.json Outdated Show resolved Hide resolved

razvanbadea-msft reviewed Sep 20, 2024

View reviewed changes

...ication/app/resource-manager/Microsoft.App/preview/2024-08-02-preview/CommonDefinitions.json Outdated Show resolved Hide resolved

razvanbadea-msft reviewed Sep 20, 2024

View reviewed changes

...ation/app/resource-manager/Microsoft.App/preview/2024-08-02-preview/ManagedEnvironments.json Outdated Show resolved Hide resolved

razvanbadea-msft added the ARMChangesRequested label Sep 20, 2024

openapi-pipeline-app bot removed the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Sep 20, 2024

Tratcher reviewed Sep 20, 2024

View reviewed changes

...ication/app/resource-manager/Microsoft.App/preview/2024-08-02-preview/CommonDefinitions.json Outdated Show resolved Hide resolved

Tratcher reviewed Sep 20, 2024

View reviewed changes

...ication/app/resource-manager/Microsoft.App/preview/2024-08-02-preview/CommonDefinitions.json Outdated Show resolved Hide resolved

tmacam reviewed Sep 20, 2024

View reviewed changes

...ication/app/resource-manager/Microsoft.App/preview/2024-08-02-preview/CommonDefinitions.json Outdated Show resolved Hide resolved

...ication/app/resource-manager/Microsoft.App/preview/2024-08-02-preview/CommonDefinitions.json Outdated Show resolved Hide resolved

resolve comments

f5ea3c0

Taher Darolywala added 2 commits October 2, 2024 01:15

add missing pattern

d04f193

swagger prettier check

2151b4a

azure-sdk removed the BreakingChange-Python-Sdk label Oct 2, 2024

Change descriptions

508abd0

tdaroly changed the title ~~Tdarolywala/advanced ingress routing~~ Advanced ingress - ACA - Swagger API Oct 3, 2024

jijohn14 reviewed Oct 4, 2024

View reviewed changes

...ication/app/resource-manager/Microsoft.App/preview/2024-08-02-preview/CommonDefinitions.json Outdated Show resolved Hide resolved

jijohn14 suggested changes Oct 4, 2024

View reviewed changes

Taher Darolywala added 3 commits October 4, 2024 13:35

move to single arm spec file, resolve comments

fe391bb

nits:

c916add

update readme, fix prettier check

fe7ba96

order

4dddaac

tdaroly closed this by deleting the head repository Oct 4, 2024

tdaroly changed the title ~~Advanced ingress - ACA - Swagger API~~ Advanced ingress - ACA - Swagger API - DELETED - NEW PR - https://github.com/Azure/azure-rest-api-specs/pull/30862 Oct 4, 2024

Conversation

tdaroly commented Sep 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a PR Template

Uh oh!

openapi-pipeline-app bot commented Sep 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Next Steps to Merge

Uh oh!

openapi-pipeline-app bot commented Sep 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Generated ApiView

Uh oh!

razvanbadea-msft commented Sep 20, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Tratcher Sep 20, 2024 • 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!

razvanbadea-msft commented Sep 20, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

openapi-pipeline-app bot commented Oct 4, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

tdaroly commented Sep 20, 2024 •

edited

Loading

openapi-pipeline-app bot commented Sep 20, 2024 •

edited

Loading

openapi-pipeline-app bot commented Sep 20, 2024 •

edited

Loading

Tratcher Sep 20, 2024 •

edited

Loading