Discovery/2026 02 01 preview cp#39745
Conversation
Next Steps to Merge✅ All automated merging requirements have been met! To get your PR merged, see aka.ms/azsdk/specreview/merge.Comment generated by summarize-checks workflow run. |
github-actions
bot
added
CI-RpaaSRPNotInPrivateRepo
new-rp-namespace
resource-manager
RPaaS
SuppressionReviewRequired
TypeSpec
API Change CheckAPIView identified API level changes in this PR and created the following API reviews
|
|
Corresponding RPSaaSMasterPR: https://github.com/Azure/azure-rest-api-specs-pr/pull/26366 |
|
Pointer to our spec files in RPSaaSMaster here. Will bypass CI-RpaaSRPNotInPrivateRepo label by adding RPaaSException label due to known tooling constraints. Tracking: [summarize-checks] Improve handling of label |
achocron
added
the
RPaaSException
There was a problem hiding this comment.
Pull request overview
This PR introduces the 2026-02-01-preview API version for the Microsoft.Discovery resource provider. The changes are implemented using TypeSpec and include new resources, private endpoint support, and storage management capabilities. The PR adds comprehensive TypeSpec definitions across multiple management domains (Workspace, Supercomputer, Bookshelf) along with example files and configuration.
Changes:
- New TypeSpec-based API version (2026-02-01-preview) with modular structure across Discovery.Management, Discovery.Workspace.Management, Discovery.Supercomputer.Management, and Discovery.Bookshelf.Management
- Introduction of StorageContainer and StorageAsset resources replacing deprecated DataContainer/DataAsset
- Addition of private endpoint and private link resource support for Workspace and Bookshelf resources
- New ChatModelDeployment resource for AI model deployments
- Comprehensive example files for all operations
Reviewed changes
Copilot reviewed 159 out of 160 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| specification/discovery/resource-manager/readme.md | AutoRest configuration file for the new API version with suppressions |
| specification/discovery/cspell.yaml | Spell checker configuration with domain-specific terms |
| specification/discovery/Discovery.Management/tspconfig.yaml | TypeSpec compiler configuration with emitter settings |
| specification/discovery/Discovery.Management/*.tsp | Core TypeSpec definitions for tools, storage, agents, workflows, models |
| specification/discovery/Discovery.Workspace.Management/*.tsp | Workspace-related resources including projects and chat model deployments |
| specification/discovery/Discovery.Supercomputer.Management/*.tsp | Supercomputer and node pool resource definitions |
| specification/discovery/Discovery.Bookshelf.Management/*.tsp | Bookshelf resource with private endpoint support |
| specification/discovery/Discovery.Management.Shared/*.tsp | Shared models and namespace definitions |
| specification/discovery/Discovery.Management/examples/**/*.json | 100+ example files for all operations |
| @doc("Environment variables to make available") | ||
| @visibility(Lifecycle.Read, Lifecycle.Create, Lifecycle.Update) | ||
| environmentVariables?: Record<string>; |
There was a problem hiding this comment.
The ToolProperties.environmentVariables field is modeled as a plain Record<string> with read visibility, so any secrets placed in these environment variables (for example, connection strings or API keys) will be returned in cleartext to any caller with read access to the tool resource. This effectively makes the ARM management API a secret exfiltration channel for tool runtime configuration, rather than using a secret store or redacted responses as other ARM services do. Consider treating these values as secrets (for example, via secret/reference types or redacted-on-read semantics) so that sensitive environment data cannot be retrieved via standard GET operations.
There was a problem hiding this comment.
understand the reason for using Record, however agree with this comment.
There was a problem hiding this comment.
approved suppression for rest of the cases in this PR.
There was a problem hiding this comment.
Thanks! Capturing offline conversation - this change is already merged to the same API version in RPSaaSMaster and is partially rolled out from our manifest. Changing this would be breaking from a spec standpoint. We also have prior API versions in private preview which use this field already.
There was a problem hiding this comment.
I will add that there is nothing in our tool that mandates these fields contains secrets. I will share a note with my team on guiding customers not to store secrets here.
There was a problem hiding this comment.
Here is the link to when this was originally introduced to RPSaaSMaster: https://github.com/Azure/azure-rest-api-specs-pr/pull/22409/changes#diff-8d184e1ec13ad060c85d86699a8a8ea4ce1a4965a5dc2dc86b479beb22ad267c
Thank you for flagging this - I spoke with my team, and indeed we will add a note on our documentation to avoid using this field for secrets as that is not its intended application. For now, we are comfortable with the API as-is given our current position and will investigate further improvements in a future API version.
There was a problem hiding this comment.
The Copilot comment above doesn't get at the real issue. Record/unschematized properties are an ARM anti-pattern. They cannot be supported by standard ARM features like Azure Policy and Azure Resource Graph that customers expect to work. They are essentially secret contracts that have no spec and bypass Azure versioning, creating difficult-to-use APIs, opportunity for documentation holes, and potential unexpected breaks.
See: https://armwiki.azurewebsites.net/api_contracts/guidelines/openapi.html#oapi025-avoid-properties-that-accept-values-of-different-types and https://armwiki.azurewebsites.net/api_contracts/guidelines/openapi.html#oapi032-only-use-additionalproperties-when-the-object-properties-are-dynamic-unknown-or-user-defined.
You can support the environment variables pattern using an array/collection of key/value pairs, which is fully supported by ARM.
There was a problem hiding this comment.
Understood, this is totally fair from a modeling standpoint, and I will communicate with my team that we need to investigate the feedback.
From a process standpoint, I just want to clarify that the intent of this PR is basically just a file copy from RPSaaSMaster to public main (but a few of the older versions are stripped out of this one).
We aren't sure how to incorporate net new feedback into the 2026-02-01-preview API version - the source of truth is actually already merged in RPSaaSMaster with ARM Approval (PR here), and we've even already rolled out this model to ARM canary already and even a few internal endpoints in production.
There was a problem hiding this comment.
@achocron - Got it. Apologies: I was slow to realize that this specific API version had previously merged to RPSaaSMaster. Assuming there are no changes between the checked-in API version files and the files in this PR, it means effectively this PR has already been signed off as-is. Adding the labels.
Please seriously consider the feedback for your next API version, particularly before your APIs become public.
There was a problem hiding this comment.
Thanks very much, and no worries, I am glad we had the valuable conversation.
The swaggers are identical, I just did some typespec surgeries to remove old versions of the API we don't want to merge.
sandipsh
added
Approved-Suppression
ARMChangesRequested
and removed
WaitForARMFeedback
github-actions
bot
added
the
WaitForARMFeedback
github-actions
bot
removed
the
WaitForARMFeedback
github-actions
bot
added
the
WaitForARMFeedback
mentat9
added
Approved-Suppression
ARMSignedOff
github-actions
bot
removed
the
WaitForARMFeedback
achocron
added
the
PublishToCustomers
* Discovery Control Plane public preview 2026-02-01-preview --------- Co-authored-by: Mark Cowlishaw <markcowl@microsoft.com> Co-authored-by: Sandip Shahane <sandipsh@microsoft.com>
ARM (Control Plane) API Specification Update Pull Request
Tip
Overwhelmed by all this guidance? See the
Getting helpsection 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!
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:
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 ApiViewcomment 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
Purpose of this PRandDue diligence checklist.write accessper aka.ms/azsdk/access#request-access-to-rest-api-or-sdk-repositoriesNext Steps to Mergecomment. It will appear within few minutes of submitting this PR and will continue to be up-to-date with current PR state.and https://aka.ms/ci-fix.
queuedstate, please add a comment with contents/azp run.This should result in a new comment denoting a
PR validation pipelinehas started and the checks should be updated after few minutes.