Convert Servicelinker Swagger to Tsp #34326

mcgallan · 2025-04-29T08:01:09Z

This PR migrates your latest version (identified by the tag in your readme.md) of swagger to TypeSpec. We already tried our best to make sure the TypeSpec represents same as previous swagger. Since we lack the business knowledge, please validate this PR again to make sure it's functional equivalent as before. The local validation step is at Getting started | TypeSpec Azure

Besides, TypeSpec encourages to follow ARM guidelines. Therefore, some representations in your previous swagger will be fixed to follow these guidelines. When you see differences in your local validation, please keep this note in mind. Here is the list you need to pay attention to:

You have several models extending from multiple base models: CreateOrUpdateDryrunParameters, ServicePrincipalSecretAuthInfo, SystemAssignedIdentityAuthInfo, UserAssignedIdentityAuthInfo

Please reach out to TypeSpec Discussions Channel if there is any help needed.

openapi-pipeline-app · 2025-04-29T08:01:21Z

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 is not ready for ARM review (label: NotReadyForARMReview). This PR will not be reviewed by ARM until relevant problems are fixed. Consult the rest of this Next Steps to Merge comment for details.
Once the blocking problems are addressed, add to the PR a comment with contents /azp run. Automation will re-evaluate this PR and if everything looks good, it will add WaitForARMFeedback label which will put this PR on the ARM review queue.
For details of the ARM review, see aka.ms/azsdk/pr-arm-review
❌ This PR is NotReadyForARMReview because it has the VersioningReviewRequired label.
❌ This PR has at least one change violating Azure versioning policy (label: VersioningReviewRequired).
To unblock this PR, either a) introduce a new API version with these changes instead of modifying an existing API version, or b) follow the process at aka.ms/brch.

openapi-pipeline-app · 2025-04-29T08:01:21Z

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

github-actions · 2025-04-29T08:04:01Z

API Change Check

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

Language	API Review for Package
Swagger	Microsoft.ServiceLinker
TypeSpec	Microsoft.ServiceLinker
Java	com.azure.resourcemanager:azure-resourcemanager-servicelinker
Go	sdk/resourcemanager/servicelinker/armservicelinker

v-jiaodi · 2025-07-22T09:54:57Z

specification/servicelinker/ServiceLinker.Management/models.tsp

+   * True if connect via Kubernetes CSI Driver.
+   */
+  #suppress "@azure-tools/typespec-azure-core/no-nullable" "FIXME: Update justification, follow aka.ms/tsp/conversion-fix for details"
+  connectAsKubernetesCsiDriver?: boolean | null;


@MaryGao Do we accept the change here?

issue track: Azure/autorest.typescript#3394

After you have a clear discussion result, I hope you can reply under this comment to help me fix the issues in the tsp in a timely manner.

@v-jiaodi I see in the discussion of another issue that this change seems to have already been accepted. Do you agree that we can close this comment now?

tadelesh · 2025-07-25T03:41:45Z

Connector's subscriptionId parameter should be in method level. TCGC issue here: Azure/typespec-azure#2682

tadelesh · 2025-07-25T05:28:20Z

Another issue in this service: duplicate enum problem: Azure/typespec-azure#3050

catalinaperalta

Some comments to improve the spec and tooling. cc @pshao25

catalinaperalta · 2025-08-01T22:11:11Z

specification/servicelinker/ServiceLinker.Management/DryrunResource.tsp

Nit: can we use the same casing for all .tsp file names in the project directory?

The same casing you mean "DryRunResource"? We honored what service defined here.

catalinaperalta · 2025-08-01T22:12:19Z

specification/servicelinker/ServiceLinker.Management/DryrunResource.tsp

+      {
+        ...ApiVersionParameter,
+
+        #suppress "@azure-tools/typespec-azure-core/documentation-required" "FIXME: Update justification, follow aka.ms/tsp/conversion-fix for details"


Nit: since this looks pretty standard we should add docs for it

There's an instance below where we did add docs for the same param, why didnt we do the same here?

docs added and suppression has been removed

@mcgallan These operations seem to be extension resource operations. Please check and do the corresponding fixes.

As we discussed in the meeting, the current TypeSpec handling of resourceUri seems to be somewhat tricky and may require waiting for a new solution

These operations have been updated to ExtensionResource.

catalinaperalta · 2025-08-01T22:13:41Z

specification/servicelinker/ServiceLinker.Management/DryrunResource.tsp

+  /**
+   * add a dryrun job to do necessary check before actual creation
+   */
+  #suppress "@azure-tools/typespec-azure-resource-manager/lro-location-header" "FIXME: Update justification, follow aka.ms/tsp/conversion-fix for details"


Is this an lro? And if so, how does it perform polling?

Yes, LRO! I didn't get your question actually. How service perform polling? I feel like it's not a question to me.

catalinaperalta · 2025-08-01T22:18:50Z

specification/servicelinker/ServiceLinker.Management/LinkerResource.tsp

+   * Delete a Connector.
+   */
+  #suppress "@azure-tools/typespec-azure-resource-manager/lro-location-header" "FIXME: Update justification, follow aka.ms/tsp/conversion-fix for details"
+  #suppress "@azure-tools/typespec-azure-resource-manager/arm-delete-operation-response-codes" "FIXME: Update justification, follow aka.ms/tsp/conversion-fix for details"


Why are we getting this linter warning with this template? I dont see it modifying the response code. If this is non-standard behavior that is purposefully added for legacy reasons, seems like the suppression should be on the template itself instead of making every user of this template add suppressions for it

It does. The Response is already customzied.

catalinaperalta · 2025-08-01T22:19:43Z

specification/servicelinker/ServiceLinker.Management/LinkerResource.tsp

+  /**
+   * Delete a Connector.
+   */
+  #suppress "@azure-tools/typespec-azure-resource-manager/lro-location-header" "FIXME: Update justification, follow aka.ms/tsp/conversion-fix for details"


Why is the location header missing? What is the polling method for this operation? Will downstream client libraries know how to perform polling with this spec?

I've no ideas what the polling operation is. I just honored what swagger defined. There is no place in swagger could identify a polling operation.

specification/servicelinker/ServiceLinker.Management/main.tsp

catalinaperalta · 2025-08-01T22:24:17Z

specification/servicelinker/ServiceLinker.Management/models.tsp

+ */
+union TargetServiceType {
+  string,
+  #suppress "@azure-tools/typespec-azure-core/documentation-required" "FIXME: Update justification, follow aka.ms/tsp/conversion-fix for details"


For enum values that dont have documentation, can our tooling just output the name of the value as docs?

A small fix like that in our tooling would greatly improve this output and reduce the number of suppressions in the specs

We can add ducoment for enum values by its enum value.

catalinaperalta · 2025-08-01T22:25:41Z

specification/servicelinker/ServiceLinker.Management/models.tsp

The documentation suppressions add a lot of noise in this file

catalinaperalta · 2025-08-01T22:30:06Z

specification/servicelinker/ServiceLinker.Management/routes.tsp

+  /**
+   * List the dapr configuration supported by Service Connector.
+   */
+  @route("/{+resourceUri}/providers/Microsoft.ServiceLinker/daprConfigurations")


What is the +resourceUri doing?

The scope parameter for an extension operation should be allowReserved.

catalinaperalta · 2025-08-01T22:30:36Z

specification/servicelinker/ServiceLinker.Management/routes.tsp

+       * OData skipToken option for pagination.
+       */
+      @query("$skipToken")
+      $skipToken?: string;


Nit: sinc eyou already have the proper name in the query decorator you can improve the tsp name

Suggested change

$skipToken?: string;

skipToken?: string;

If we have tooling doing this, it would be worth a look at improving this

This would introduce swagger diff, which is a cost for us to do the comparison.

I need to evaluate the impact to SDK.

I need to do some investigation why there is a $.

github-actions · 2025-08-21T05:56:06Z

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 is not ready for ARM review (label: NotReadyForARMReview). This PR will not be reviewed by ARM until relevant problems are fixed. Consult the rest of this Next Steps to Merge comment for details.
Once the blocking problems are addressed, add to the PR a comment with contents /azp run. Automation will re-evaluate this PR and if everything looks good, it will add WaitForARMFeedback label which will put this PR on the ARM review queue.
For details of the ARM review, see aka.ms/azsdk/pr-arm-review
❌ This PR is NotReadyForARMReview because it has the VersioningReviewRequired label.
❌ This PR has at least one change violating Azure versioning policy (label: VersioningReviewRequired).
To unblock this PR, either a) introduce a new API version with these changes instead of modifying an existing API version, or b) follow the process at aka.ms/brch.
❌ The required check named TypeSpec Validation 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

Comment generated by summarize-checks workflow run.

mcgallan · 2025-08-22T06:36:30Z

Another issue in this service: duplicate enum problem: Azure/typespec-azure#3050

I see that in this PR you have already used clientLocation to fix this issue. Is this a widely accepted way to fix it? Because this kind of problem also occurs in other swagger projects, perhaps this approach can be used to solve these problems.

mcgallan added 2 commits April 29, 2025 15:31

convert generated and fixed some error

d0ddf99

generate swagger

90045ba

AzureRestAPISpecReview added resource-manager RPaaS TypeSpec Authored with TypeSpec labels Apr 29, 2025

mcgallan added 12 commits May 6, 2025 15:51

Merge remote-tracking branch 'upstream/main' into Convert-Servicelinker

a9ac2a0

generate diff

9389cb6

Merge remote-tracking branch 'upstream/main' into Convert-Servicelinker

ee2b24e

Merge remote-tracking branch 'upstream/main' into Convert-Servicelinker

90447bf

fix

16439d7

update

5d0721d

update

ca12b7b

update

4b5dba1

update

6204228

update

9c08008

Merge remote-tracking branch 'upstream/main' into Convert-Servicelinker

f16e1cc

update

4039c1d

AzureRestAPISpecReview added the VersioningReviewRequired <valid label in PR review process>add this label when versioning review is required label May 21, 2025

mcgallan and others added 5 commits May 21, 2025 17:21

Merge branch 'main' into Convert-Servicelinker

89e683b

update

c11983a

Merge branch 'main' into Convert-Servicelinker

f6e832e

update

783f394

update

feed1fc

v-jiaodi reviewed Jul 22, 2025

View reviewed changes

tadelesh added 2 commits July 25, 2025 13:28

go customization

de7befb

rename duplicate enum

9c14341

catalinaperalta reviewed Aug 1, 2025

View reviewed changes

MaryGao mentioned this pull request Aug 5, 2025

Should we translate x-nullable to | null for Azure services? Azure/typespec-azure#3121

Closed

Merge branch 'main' into Convert-Servicelinker

a9d2931

mcgallan added the rpaas-tsp-conversion label Aug 12, 2025

mcgallan and others added 6 commits August 12, 2025 11:38

Merge branch 'main' into Convert-Servicelinker

ffbfe26

update

ea0ee07

fix nextlinkname

a4052c4

Merge branch 'main' into Convert-Servicelinker

83c96cb

removed useDependency

dca4e65

Merge branch 'main' into Convert-Servicelinker

c920b74

pshao25 mentioned this pull request Aug 22, 2025

TypeSpec migration post items from reviewers Azure/typespec-azure#3190

Open

mcgallan added 3 commits August 22, 2025 14:44

update docs for resourceUri parameter

5c97657

remove suppression

e3e66a0

update ExtensionResource in ServiceLinker

e8756ae

josefree mentioned this pull request Aug 28, 2025

test breaking change cross version #35922

Draft

msyyc mentioned this pull request Sep 11, 2025

[Typespec migration] move subId to method level to avoid SDK breaking Azure/typespec-azure#3260

Open

3 tasks

msyyc and others added 3 commits September 11, 2025 16:42

Merge branch 'main' into Convert-Servicelinker

92394ac

fix flatten

3c38f07

Merge branch 'main' into Convert-Servicelinker

0e3900b

HarveyLink mentioned this pull request Sep 23, 2025

[ServiceLinker] SDK validation for TypeSpec Azure/azure-sdk-for-net#52773

Open

microsoft-github-policy-service bot added no-recent-activity There has been no recent activity on this issue. and removed no-recent-activity There has been no recent activity on this issue. labels Oct 6, 2025

downgrade sub id parameter

793bd2c

Convert Servicelinker Swagger to Tsp #34326

Are you sure you want to change the base?

Convert Servicelinker Swagger to Tsp #34326

Conversation

mcgallan commented Apr 29, 2025 • edited by pshao25 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openapi-pipeline-app bot commented Apr 29, 2025 • 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 Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

API Change Check

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!

tadelesh commented Jul 25, 2025

Uh oh!

tadelesh commented Jul 25, 2025

Uh oh!

catalinaperalta left a comment

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!

pshao25 Aug 26, 2025 • 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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

pshao25 Aug 4, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

mcgallan commented Apr 29, 2025 •

edited by pshao25

Loading

openapi-pipeline-app bot commented Apr 29, 2025 •

edited

Loading

openapi-pipeline-app bot commented Apr 29, 2025 •

edited

Loading

github-actions bot commented Apr 29, 2025 •

edited

Loading

pshao25 Aug 26, 2025 •

edited

Loading

pshao25 Aug 4, 2025 •

edited

Loading

pshao25 Aug 20, 2025 •

edited

Loading

github-actions bot commented Aug 21, 2025 •

edited

Loading