[cplat-2024-03-03] Add inVMAccessControlProfile spec in GalleryRP #30504

lkumaar · 2024-09-11T05:12:43Z

ARM (Control Plane) API Specification Update Pull Request

Tip

Overwhelmed by all this guidance? See the Getting help section at the bottom of this PR description.

PR review workflow diagram

Please understand this diagram before proceeding. It explains how to get your PR approved & merged.

Purpose of this PR

What's the purpose of this PR? Check the specific option that applies. This is mandatory!

New resource provider.
New API version for an existing resource provider. (If API spec is not defined in TypeSpec, the PR should have been created in adherence to OpenAPI specs PR creation guidance).
Update existing version for a new feature. (This is applicable only when you are revising a private preview API version.)
Update existing version to fix OpenAPI spec quality issues in S360.
Convert existing OpenAPI spec to TypeSpec spec (do not combine this with implementing changes for a new API version).
Other, please clarify:
- edit this with your clarification

Due diligence checklist

To merge this PR, you must go through the following checklist and confirm you understood
and followed the instructions by checking all the boxes:

I confirm this PR is modifying Azure Resource Manager (ARM) related specifications, and not data plane related specifications.
I have reviewed following Resource Provider guidelines, including
ARM resource provider contract and
REST guidelines (estimated time: 4 hours).
I understand this is required before I can proceed to the diagram Step 2, "ARM API changes review", for this PR.

Additional information

Viewing API changes

For convenient view of the API changes made by this PR, refer to the URLs provided in the table
in the Generated ApiView comment added to this PR. You can use ApiView to show API versions diff.

Suppressing failures

If one or multiple validation error/warning suppression(s) is detected in your PR, please follow the
suppressions guide to get approval.

Getting help

First, please carefully read through this PR description, from top to bottom. Please fill out the Purpose of this PR and Due diligence checklist.
If you don't have permissions to remove or add labels to the PR, request write access per aka.ms/azsdk/access#request-access-to-rest-api-or-sdk-repositories
To understand what you must do next to merge this PR, see the Next Steps to Merge comment. It will appear within few minutes of submitting this PR and will continue to be up-to-date with current PR state.
For guidance on fixing this PR CI check failures, see the hyperlinks provided in given failure
and https://aka.ms/ci-fix.
For help with ARM review (PR workflow diagram Step 2), see https://aka.ms/azsdk/pr-arm-review.
If the PR CI checks appear to be stuck in queued state, please add a comment with contents /azp run.
This should result in a new comment denoting a PR validation pipeline has started and the checks should be updated after few minutes.
If the help provided by the previous points is not enough, post to https://aka.ms/azsdk/support/specreview-channel and link to this PR.

openapi-pipeline-app · 2024-09-11T05:12:47Z

Next Steps to Merge

✅ All automated merging requirements have been met! To get your PR merged, see aka.ms/azsdk/specreview/merge.

openapi-pipeline-app · 2024-09-11T05:12:47Z

Generated ApiView

Language	Package Name	ApiView Link
Go	sdk/resourcemanager/compute/armcompute	https://apiview.dev/Assemblies/Review/13fa09201aa545c19d240f82d38c09c9?revisionId=416c3195caa8418f808605ab22e75468
JavaScript	@azure/arm-compute	https://apiview.dev/Assemblies/Review/a802869b6345406cbb6d1e487132d7dd?revisionId=d9bbf0f7b29d463bbb692d8d94b5c0a8
.Net	Azure.ResourceManager.Compute	https://apiview.dev/Assemblies/Review/7fec998c6beb4201bb99c335d06c70fa?revisionId=3556b71a0ad242f88b4d2916a60f7bc1
Java	azure-resourcemanager-compute-generated	https://apiview.dev/Assemblies/Review/eb099b2efb0f4373a854e09003a6b31d?revisionId=98d859811ffc43118de9b4e1c05f357b
Swagger	Microsoft.Compute	https://apiview.dev/Assemblies/Review/b32a43d6ad48477b9343d7a921e72411?revisionId=46b3f06281304f3eace2e1f9a2ddcdfe

lkumaar · 2024-09-11T05:13:41Z

ARM sign off received in #29852

lkumaar · 2024-09-11T06:22:52Z

/azp run

azure-pipelines · 2024-09-11T06:22:57Z

Commenter does not have sufficient privileges for PR 30504 in repo Azure/azure-rest-api-specs

grizzlytheodore · 2024-09-11T16:07:14Z

please resolve lintDiff CI failures @lkumaar

lkumaar · 2024-09-12T10:03:09Z

@grizzlytheodore I've resolved most LintDiff errors, there are two sets remaining:

PATCH related rules - our services have always treated PUT and PATCH as the same (same controller, PUT supports patch semantics). I think this is standard across compute.
AvoidAdditionalProperties - the queryParameters property is a CaseInsensitiveDictionary. I'm not sure how this can be modelled without the use of AdditionalProperties

would it be possible to bypass these rules?

lkumaar · 2024-09-19T17:13:29Z

@grizzlytheodore I've resolved most LintDiff errors, there are two sets remaining:

PATCH related rules - our services have always treated PUT and PATCH as the same (same controller, PUT supports patch semantics). I think this is standard across compute.

AvoidAdditionalProperties - the queryParameters property is a CaseInsensitiveDictionary. I'm not sure how this can be modelled without the use of AdditionalProperties

would it be possible to bypass these rules?

I've added suppressions for these rules, requesting a review.

viveksheelsingh · 2024-09-25T02:59:46Z

...fication/compute/resource-manager/Microsoft.Compute/GalleryRP/stable/2024-03-03/gallery.json

+            "required": true,
+            "type": "string",
+            "pattern": "^[a-zA-Z0-9]+([-._]?[a-zA-Z0-9]+)*$",
+            "description": "The name of the gallery inVMAccessControlProfile to be created or updated. The allowed characters are alphabets and numbers with dots, dashes, and periods allowed in the middle. The maximum length is 80 characters."


can use MaxLength property

viveksheelsingh · 2024-09-25T03:07:18Z

...fication/compute/resource-manager/Microsoft.Compute/GalleryRP/stable/2024-03-03/gallery.json

+            "description": "Parameters supplied to the update gallery inVMAccessControlProfile operation."
+          }
+        ],
+        "responses": {


Pl refer to: https://github.com/Azure/azure-resource-manager-rpc/blob/master/v1.0/resource-api-reference.md#response-1 for guidelines on sync/async completion of a PATCH call...

200 for sync, 202 for async... I get that.

but considering we have always returned 200 (e.g) for our LRO patch APIs, it would not be possible to change this behaviour now. hence, we are requesting for the suppression

in fact, we had a swagger review for this new API ~2 months back in #29852 . neither the CI nor the reviewer raised this concern, so this behaviour in our service code is already in production

Do you know if your SDK tracks an async PATCH properly ? It may be the case that PATCH is completing Sync most of the time so terminating with a 200 and No need to use the LRO tracking...

Even if the SDK tracks properly for 200+async behaviour - how will consumers of direct REST API work ? Normally there is no LRO with 200 - so this is confusing

@pratikjoshi14 @sorabhtandon -- what do you folks reckon ?

I can check on 1.

For 2. if direct REST API is used, users will need to check headers for Azure-AsyncOperation, regardless of if it is 200 or 202 right?

Semantically, since 200 is terminal, I dont think it makes sense for the client to check the headers to try and poll...

@viveksheelsingh I discussed with my team, PUT for our APIs follow patch semantics and there is functionally no difference between processing a PUT request vs a PATCH request for an existing resource (same controller in the service code)

I have removed PATCH from the spec, but it seems like it breaks another LintDiff rule TrackedResourcePatchOperation. since our PUT allows for patch-like updates, can we get a suppression for this rule? or do certain clients need a PATCH api?

reverting last commit to discuss this over email

viveksheelsingh · 2024-09-25T03:08:30Z

...fication/compute/resource-manager/Microsoft.Compute/GalleryRP/stable/2024-03-03/gallery.json

+            "$ref": "../../../common-types/v1/common.json#/parameters/ApiVersionParameter"
+          }
+        ],
+        "responses": {


Pl refer to https://github.com/Azure/azure-resource-manager-rpc/blob/master/v1.0/resource-api-reference.md#response-2 for delete response guidelines

this delete is an LRO, and we have 202 and 204 as the response codes which is what the reference also suggests. am I missing something?

sure, for always async delete this is fine, if you dont have a sync path.

yes, this is async only

This reverts commit 4b884a3.

* copy last version folder as 2024-03-03 * change version reference in swagger json files and examples * add new readme.md tag with new version * [cplat-2024-03-03] Add inVMAccessControlProfile spec in GalleryRP (#30504) * Remove async responses for HTTP 200 and 204 in delete examples * Add spec for inVMAccessControlProfile * Change put to get for list inVMAccess endpoints * LintDiff - remove 202 for PUT LRO * LintDiff - add provisioningState for GalleryInVMAccessControlProfile * LintDiff - remove 200 for DELETE LRO * LintDiff - add missing type:object for new definitions * LintDiff - add regex pattern for resource names * LintDiff - add regex pattern for resource names * LintDiff - add regex pattern for resource names * LintDiff - add Location and Azure-AsyncOperation headers for 202 LROs * Add LintDiff suppressions for PATCH operations * Add LintDiff suppressions for AvoidAdditionalProperties * Update suppression reason for AvoidAdditionalProperties * Add maxLength for inVMAccessControlProfile name * Remove PATCH spec and PATCH suppressions * Revert "Remove PATCH spec and PATCH suppressions" This reverts commit 4b884a3. * Update spec with blockDeletionBeforeEndOfLife (#30505) * [cplat-2024-03-03] Update spec for "Update GalleryImageDefinition Feature" feature (#30536) * Update spec with startsAtVersion * Add allowUpdateImage property * [cplat-2024-03-03] Update spec for SoftDelete Restore feature (#30558) * Update spec with restore * Add example for soft delete restore * Update spec with scriptBehaviorAfterReboot (#30842) * Adding AdditionalReplicaSet parameter in swagger api version 2024-03-03 (#30855) * adding additionalReplicaSet in swagger * addressing lindiff errors * addressing comments * addressing comments * updating enum * running prettier * adding separate examples * adding file in readme.md * adding at the end of properties * addressing comments * Update spec with MSI support for Galleries (#30885) * Initial commit * Modify PUT/PATCH response * Fixes * Fixes * [cplat-2024-03-03] Add List soft-deleted resources api (#31094) * Add List soft-deleted resources api and example * fix bugs * [cplat-2024-03-03] Update spec for ValidationProfile (#30658) * Update spec with validationsProfile * Add examples for GetGalleryImageVersion with ValidationProfile * Run jq to format for prettier * LintDiff - add type:object for new definitions * Reorder validationsProfile to preserve param ordering * Make validations profile readonly * Update gallery.json --------- Co-authored-by: Theodore Chang <thchan@microsoft.com> * suppress lintDiff existing from last version * Move validationsProfile to GalleryImageVersionProperties (#31310) * Move validationsProfile to GalleryImageVersionProperties * Revert "Move validationsProfile to GalleryImageVersionProperties" This reverts commit ae39672. * Move validationsProfile to GalleryImageVersionProperties * Remove validationsProfile * Revert "Remove validationsProfile" This reverts commit 36ca3e0. * fix lintDiff failure * update disclaimer (#31430) * ARM review comment addressing * Make validation status an enum (#31526) * Make validation status an enum * Run prettier * Update description of platformAttributes (#31525) * Update description of platformAttributes * Run prettier * Add readOnly: true * update softDeletedArtifactType to enum * run prettier --------- Co-authored-by: Theodore Chang <theodore.l.chang@gmail.com> Co-authored-by: lkumaar <168636550+lkumaar@users.noreply.github.com> Co-authored-by: kgulshan827 <56105750+kgulshan827@users.noreply.github.com> Co-authored-by: mayankdaruka-msft <133302048+mayankdaruka-msft@users.noreply.github.com> Co-authored-by: jackyChenAzure <131018173+jackyChenAzure@users.noreply.github.com>

* copy last version folder as 2024-03-03 * change version reference in swagger json files and examples * add new readme.md tag with new version * [cplat-2024-03-03] Add inVMAccessControlProfile spec in GalleryRP (Azure#30504) * Remove async responses for HTTP 200 and 204 in delete examples * Add spec for inVMAccessControlProfile * Change put to get for list inVMAccess endpoints * LintDiff - remove 202 for PUT LRO * LintDiff - add provisioningState for GalleryInVMAccessControlProfile * LintDiff - remove 200 for DELETE LRO * LintDiff - add missing type:object for new definitions * LintDiff - add regex pattern for resource names * LintDiff - add regex pattern for resource names * LintDiff - add regex pattern for resource names * LintDiff - add Location and Azure-AsyncOperation headers for 202 LROs * Add LintDiff suppressions for PATCH operations * Add LintDiff suppressions for AvoidAdditionalProperties * Update suppression reason for AvoidAdditionalProperties * Add maxLength for inVMAccessControlProfile name * Remove PATCH spec and PATCH suppressions * Revert "Remove PATCH spec and PATCH suppressions" This reverts commit 4b884a3. * Update spec with blockDeletionBeforeEndOfLife (Azure#30505) * [cplat-2024-03-03] Update spec for "Update GalleryImageDefinition Feature" feature (Azure#30536) * Update spec with startsAtVersion * Add allowUpdateImage property * [cplat-2024-03-03] Update spec for SoftDelete Restore feature (Azure#30558) * Update spec with restore * Add example for soft delete restore * Update spec with scriptBehaviorAfterReboot (Azure#30842) * Adding AdditionalReplicaSet parameter in swagger api version 2024-03-03 (Azure#30855) * adding additionalReplicaSet in swagger * addressing lindiff errors * addressing comments * addressing comments * updating enum * running prettier * adding separate examples * adding file in readme.md * adding at the end of properties * addressing comments * Update spec with MSI support for Galleries (Azure#30885) * Initial commit * Modify PUT/PATCH response * Fixes * Fixes * [cplat-2024-03-03] Add List soft-deleted resources api (Azure#31094) * Add List soft-deleted resources api and example * fix bugs * [cplat-2024-03-03] Update spec for ValidationProfile (Azure#30658) * Update spec with validationsProfile * Add examples for GetGalleryImageVersion with ValidationProfile * Run jq to format for prettier * LintDiff - add type:object for new definitions * Reorder validationsProfile to preserve param ordering * Make validations profile readonly * Update gallery.json --------- Co-authored-by: Theodore Chang <thchan@microsoft.com> * suppress lintDiff existing from last version * Move validationsProfile to GalleryImageVersionProperties (Azure#31310) * Move validationsProfile to GalleryImageVersionProperties * Revert "Move validationsProfile to GalleryImageVersionProperties" This reverts commit ae39672. * Move validationsProfile to GalleryImageVersionProperties * Remove validationsProfile * Revert "Remove validationsProfile" This reverts commit 36ca3e0. * fix lintDiff failure * update disclaimer (Azure#31430) * ARM review comment addressing * Make validation status an enum (Azure#31526) * Make validation status an enum * Run prettier * Update description of platformAttributes (Azure#31525) * Update description of platformAttributes * Run prettier * Add readOnly: true * update softDeletedArtifactType to enum * run prettier --------- Co-authored-by: Theodore Chang <theodore.l.chang@gmail.com> Co-authored-by: lkumaar <168636550+lkumaar@users.noreply.github.com> Co-authored-by: kgulshan827 <56105750+kgulshan827@users.noreply.github.com> Co-authored-by: mayankdaruka-msft <133302048+mayankdaruka-msft@users.noreply.github.com> Co-authored-by: jackyChenAzure <131018173+jackyChenAzure@users.noreply.github.com>

lkumaar added 3 commits September 10, 2024 23:07

Remove async responses for HTTP 200 and 204 in delete examples

3de4095

Add spec for inVMAccessControlProfile

5fc9b15

Change put to get for list inVMAccess endpoints

700f20b

lkumaar requested review from Sandido, dkulkarni-ms, haagha, MS-syh2qs and grizzlytheodore as code owners September 11, 2024 05:12

microsoft-github-policy-service bot added the Compute label Sep 11, 2024

AzureRestAPISpecReview added the resource-manager label Sep 11, 2024

azure-sdk mentioned this pull request Sep 11, 2024

[AutoPR azure-mgmt-compute] [cplat-2024-03-03] Add inVMAccessControlProfile spec in GalleryRP azure-sdk/azure-sdk-for-python#8628

Closed

lkumaar added 8 commits September 12, 2024 12:31

LintDiff - remove 202 for PUT LRO

d10daf0

LintDiff - add provisioningState for GalleryInVMAccessControlProfile

25a2948

LintDiff - remove 200 for DELETE LRO

a0d8a5f

LintDiff - add missing type:object for new definitions

63979cd

LintDiff - add regex pattern for resource names

8cbf2dd

LintDiff - add regex pattern for resource names

e377a01

LintDiff - add regex pattern for resource names

8027b99

LintDiff - add Location and Azure-AsyncOperation headers for 202 LROs

d8c03b8

lkumaar added 2 commits September 19, 2024 15:56

Add LintDiff suppressions for PATCH operations

31c6172

Add LintDiff suppressions for AvoidAdditionalProperties

b0343e2

AzureRestAPISpecReview added the SuppressionReviewRequired label Sep 19, 2024

viveksheelsingh added the AzCoreIDC label Sep 24, 2024

Update suppression reason for AvoidAdditionalProperties

462bbcd

viveksheelsingh reviewed Sep 25, 2024

View reviewed changes

lkumaar added 3 commits September 26, 2024 12:36

Add maxLength for inVMAccessControlProfile name

2fe70d5

Remove PATCH spec and PATCH suppressions

4b884a3

Revert "Remove PATCH spec and PATCH suppressions"

a1838a3

This reverts commit 4b884a3.

ajaygit158 added the ARMReview label Oct 3, 2024

ajaygit158 approved these changes Oct 4, 2024

View reviewed changes

ajaygit158 added Approved-Suppression ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review labels Oct 4, 2024

grizzlytheodore approved these changes Oct 7, 2024

View reviewed changes

grizzlytheodore merged commit 7c0e2b1 into Azure:feature/cplat-2024-03-03 Oct 28, 2024
25 of 27 checks passed

grizzlytheodore mentioned this pull request Oct 30, 2024

Compute GalleryRP 2024-03-03 version #31254

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[cplat-2024-03-03] Add inVMAccessControlProfile spec in GalleryRP #30504

[cplat-2024-03-03] Add inVMAccessControlProfile spec in GalleryRP #30504

lkumaar commented Sep 11, 2024

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

edited

Loading

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

edited

Loading

lkumaar commented Sep 11, 2024

lkumaar commented Sep 11, 2024

azure-pipelines bot commented Sep 11, 2024

grizzlytheodore commented Sep 11, 2024

lkumaar commented Sep 12, 2024

lkumaar commented Sep 19, 2024

viveksheelsingh Sep 25, 2024

lkumaar Sep 26, 2024

viveksheelsingh Sep 25, 2024

lkumaar Sep 26, 2024

lkumaar Sep 26, 2024

viveksheelsingh Sep 26, 2024

lkumaar Sep 26, 2024

viveksheelsingh Sep 26, 2024

lkumaar Sep 26, 2024

lkumaar Sep 26, 2024

viveksheelsingh Sep 25, 2024

lkumaar Sep 26, 2024

viveksheelsingh Sep 26, 2024

lkumaar Sep 26, 2024

[cplat-2024-03-03] Add inVMAccessControlProfile spec in GalleryRP #30504

[cplat-2024-03-03] Add inVMAccessControlProfile spec in GalleryRP #30504

Conversation

lkumaar commented Sep 11, 2024

ARM (Control Plane) API Specification Update Pull Request

PR review workflow diagram

Purpose of this PR

Due diligence checklist

Additional information

Getting help

openapi-pipeline-app bot commented Sep 11, 2024 • edited Loading

Next Steps to Merge

openapi-pipeline-app bot commented Sep 11, 2024 • edited Loading

Generated ApiView

lkumaar commented Sep 11, 2024

lkumaar commented Sep 11, 2024

azure-pipelines bot commented Sep 11, 2024

grizzlytheodore commented Sep 11, 2024

lkumaar commented Sep 12, 2024

lkumaar commented Sep 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

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

edited

Loading

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

edited

Loading