Skip to content

SRP Jan25 [2025-01-01] API Version Swagger Changes #33529

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

Open
wants to merge 40 commits into
base: main
Choose a base branch
from

Conversation

ajashant-msft
Copy link
Member

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.

Click here to open a PR for only SDK configuration.

@github-actions github-actions bot added the brownfield Brownfield services will soon be required to convert to TypeSpec. See https://aka.ms/azsdk/typespec. label Mar 27, 2025
@microsoft-github-policy-service microsoft-github-policy-service bot added the Storage Storage Service (Queues, Blobs, Files) label Mar 27, 2025
@azure-sdk
Copy link
Collaborator

azure-sdk commented Mar 27, 2025

API Change Check

APIView identified API level changes in this PR and created the following API reviews

Language API Review for Package
Swagger Microsoft.Storage

Copy link
Member

@blueww blueww left a comment

Choose a reason for hiding this comment

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

Besides my other comments, please don't merge the PR before this API version is really available in all regions (include mooncake, usgov, ....)

Copy link

openapi-pipeline-app bot commented Mar 27, 2025

Next Steps to Merge

Next steps that must be taken to merge this PR:
  • ❌ This PR targets either the main branch of the public specs repo or the RPSaaSMaster branch of the private specs repo. These branches are not intended for iterative development. Therefore, you must acknowledge you understand that after this PR is merged, the APIs are considered shipped to Azure customers. Any further attempts at in-place modifications to the APIs will be subject to Azure's versioning and breaking change policies. Additionally, for control plane APIs, you must acknowledge that you are following all the best practices documented by ARM at aka.ms/armapibestpractices. If you do intend to release the APIs to your customers by merging this PR, add the PublishToCustomers label to your PR in acknowledgement of the above. Otherwise, retarget this PR onto a feature branch, i.e. with prefix release- (see aka.ms/azsdk/api-versions#release--branches).
  • ❌ 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 Swagger PrettierCheck has failed. 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

Copy link

openapi-pipeline-app bot commented Mar 27, 2025

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

@blueww
Copy link
Member

blueww commented Apr 1, 2025

A SRP swagger PR is merged with 2024-01-01 API change.
Please rebase this PR with that change.

#33569

@microsoft-github-policy-service microsoft-github-policy-service bot added the no-recent-activity There has been no recent activity on this issue. label Apr 21, 2025
@ajashant-msft
Copy link
Member Author

Hi, @@ajashant-msft. Your PR has no update for 14 days and it is marked as stale PR. If no further update for over 14 days, the bot will close the PR. If you want to refresh the PR, please remove no-recent-activity label.

@microsoft-github-policy-service microsoft-github-policy-service bot removed the no-recent-activity There has been no recent activity on this issue. label Apr 21, 2025
@ajashant-msft ajashant-msft reopened this Apr 21, 2025
Copy link

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

@AzureRestAPISpecReview AzureRestAPISpecReview added ARMReview BreakingChangeReviewRequired <valid label in PR review process>add this label when breaking change review is required labels May 17, 2025
ajashant-msft and others added 7 commits June 4, 2025 09:50
**Code Review Checklist**

*Code Style/Hygiene*
- [ ] Is the change self-contained ? If not, can this be broken to separate PRs ?
- [ ] If this is a new feature, is the entire feature behind a config or allow-listed subscription or AFEC ? Ensure that the feature is turned off by default and enabled only for a select set of subscriptions.
- [ ] If a new feature, and AFEC/subscription opt-in is not feasible, can this change be limited to stage/canaries and phased open in a future rollout ?
- [ ] Are configs/DCs included for variable limits, settings and to disable the feature off as a whole ?
- [ ] If a bug is being fixed that impacts written KVS state, is there a backfill (backfill job) plan in place to repair existing KVS records with bad data ? If not, is the bug fix backward compatible with existing data ?

*API versioning*
- [ ] Are new customer facing APIs or properties being introduced ? Should this change be introduced in a new API version ?
- [ ] Is any existing KVS schema being modified ? If yes, this change should be introduced in a new API version.
- [ ] If KVS schema is being modified for persistent state, is there a backfill (backfill job) plan in place to repair existing KVS records with the old schema ? If not, is the change backward compatible with existing KVS data ?

*Logging*
- [ ] Do any of the log messages include PII or sensitive information eg. keys (account/encryption), customer data (tags) ?
- [ ] Are appropriate verbose logs included to diagnose customer issues when this change is deployed ? i.e. are we able to describe a timeline of what happened to a customer request while triggering the new code path ?

*Exception/Error Handling*
- [ ] Are all non-recoverable errors handled and logged ? Is a user-friendly/actionable error code returned to the user ?
- [ ] If non-recoverable error are seen, is the transaction unwound to a clean state to avoid orphaned metadata ?
- [ ] If recoverable, are the retries bounded for non-transactional ?
- [ ] If recoverable, are the retries unbounded for transactional ?

*Memory*
- [ ] Are there any unbounded KVS reads eg. list entire KVS tables ? Consider pointed reads by key or paged reads.
- [ ] Does the change introduce an increased memory footprint to RSRP replicas ? Do we require memory profiling in preprod/test ?

*Performance*
- [ ] KVS writes cost 5-15ms. Are there any redundant KVS reads/writes ?
- [ ] Do we need to consider secondary indexes to improve operation performance ?

*Unit Test Coverage*
- [ ] Do unit tests cover all happy path & failure scenarios ?
- [ ] Do any of the unit tests introduced rely on randomness or prone to transient failures eg. time skew, randomly selecting account type or request parameter etc ?
- [ ] Do the Unit tests take less than 100ms(each) to execute when the test suite is run?
- [ ] Do any of the unit tests take a dependency on time, I/O, network call, disks etc?

*Monitoring*
- [ ] Are metrics created for outbound/inbound external dependenc...
…operty for SMB & NFS in 2025-01-01

**Objective**: To add the swagger for a new file service property - 'EncryptionInTransit' for SMB & NFS, for service version '2026-02-06' .

- The example is added in a new file S:\Azure\Storage\Storage-SRP\src\Swagger\Microsoft.Storage\stable\2026-02-06\examples\FileServicesPut_EncryptionInTransitRequired.json

**Code Review Checklist**

*Code Style/Hygiene*
- [ ] Is the change self-contained ? If not, can this be broken to separate PRs ?
- [ ] If this is a new feature, is the entire feature behind a config or allow-listed subscription or AFEC ? Ensure that the feature is turned off by default and enabled only for a select set of subscriptions.
- [ ] If a new feature, and AFEC/subscription opt-in is not feasible, can this change be limited to stage/canaries and phased open in a future rollout ?
- [ ] Are configs/DCs included for variable limits, settings and to disable the feature off as a whole ?
- [ ] If a bug is being fixed that impacts written KVS state, is there a backfill (backfill job) plan in place to repair existing KVS records with bad data ? If not, is the bug fix backward compatible with existing data ?

*API versioning*
- [ ] Are new customer facing APIs or properties being introduced ? Should this change be introduced in a new API version ?
- [ ] Is any existing KVS schema being modified ? If yes, this change should be introduced in a new API version.
- [ ] If KVS schema is being modified for persistent state, is there a backfill (backfill job) plan in place to repair existing KVS records with the old schema ? If not, is the change backward compatible with existing KVS data ?

*Logging*
- [ ] Do any of the log messages include PII or sensitive information eg. keys (account/encryption), customer data (tags) ?
- [ ] Are appropriate verbose logs included to diagnose customer issues when this change is deployed ? i.e. are we able to describe a timeline of what happened to a customer request while triggering the new code path ?

*Exception/Error Handling*
- [ ] Are all non-recoverable errors handled and logged ? Is a user-friendly/actionable error code returned to the user ?
- [ ] If non-recoverable error are seen, is the transaction unwound to a clean state to avoid orphaned metadata ?
- [ ] If recoverable, are the retries bounded for non-transactional ?
- [ ] If recoverable, are the retries unbounded for transactional ?

*Memory*
- [ ] Are there any unbounded KVS reads eg. list entire KVS tables ? Consider pointed reads by key or paged reads.
- [ ] Does the change introduce an increased memory footprint to RSRP replicas ? Do we require memory profiling in preprod/test ?

*Performance*
- [ ] KVS writes cost 5-15ms. Are there any redundant KVS reads/writes ?
- [ ] Do we need to consider secondary indexes to improve operation performance ?

*Unit Test Coverage*
- [ ] Do unit tests cover all happy path & failure scenarios ?
- [ ] Do any of the unit tests introduced rely on randomness or prone to transient failures eg. time skew, ...
**Why do we need this change?**
We missed the chance to document restore storage account Api, though put storage account operation.
This is also causing challenge of build-in policy for related scenario.
Task 29330514: Build-in policy Storage_NetworkAcls_Audit.json should consider storage account recovery scenario

----
API change

This pull request updates the Swagger documentation to include the restore storage account scenario.
- Added `DeletedStorageAccountRestore.json` example for restoring a deleted storage account.
- Updated `storage.json` to include `deletedAccountCreationTime` property and reference the new restore example.
- Modified the description for the 202 response to include restore storage account requests.
<!-- GitOpsUserAgent=GitOps.Apps.Server.pullrequestcopilot -->

Related work items: #31884198
@AzureRestAPISpecReview AzureRestAPISpecReview added the ReadyForApiTest <valid label in PR review process>add this label when swagger and service APIs are ready for test label Jun 5, 2025
@ajashant-msft
Copy link
Member Author

Fix errors from Swagger LintDiff check

done

@ajashant-msft ajashant-msft removed the ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review label Jun 5, 2025
@openapi-pipeline-app openapi-pipeline-app bot added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Jun 5, 2025
@AzureRestAPISpecReview AzureRestAPISpecReview removed the ReadyForApiTest <valid label in PR review process>add this label when swagger and service APIs are ready for test label Jun 6, 2025
@rkmanda
Copy link
Member

rkmanda commented Jun 6, 2025

There are still a bunch of errors here - https://github.com/Azure/azure-rest-api-specs/pull/33529/checks?check_run_id=43585557082

Please either fix them or provide a justification for wh these cant be fixed

@rkmanda rkmanda added the ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review label Jun 6, 2025
@openapi-pipeline-app openapi-pipeline-app bot removed the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Jun 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review ARMReview BreakingChange-Approved-BugFix Changes are to correct the REST API definition to correctly describe service behavior BreakingChange-Go-Sdk-Suppression BreakingChange-Go-Sdk-Suppression-Approved BreakingChange-JavaScript-Sdk-Approved BreakingChangeReviewRequired <valid label in PR review process>add this label when breaking change review is required brownfield Brownfield services will soon be required to convert to TypeSpec. See https://aka.ms/azsdk/typespec. new-api-version resource-manager RPaaS Storage Storage Service (Queues, Blobs, Files)
Projects
None yet
Development

Successfully merging this pull request may close these issues.