Refactor TypeSpec with separate update models by mbarnes · Pull Request #6462 · Azure/ARO-HCP

Matthew Barnes (mbarnes) · 2026-08-06T17:36:16Z

What

Refactors the TypeSpec API definition files for public preview compliance.

Primarily, we've been instructed to discontinue use of the ArmResourcePatchAsync template in favor of ArmCustomPatchAsync.

This sucks.

ArmResourcePatchAsync is nice because it auto-generates update model definitions in the JSON Swagger files. But it doesn't provide that benefit for generated SDKs and we've been largely disregarding SDK generation until now.

ArmCustomPatchAsync requires a separate set of models for patch operations. These "update" models only include properties with Lifecycle.Update visibility. This means we now have to maintain two copies of most of our TypeSpec models: one for PUT, another for PATCH.

Testing

No new tests necessary. If I did this right then current tests should all pass.

Special notes for your reviewer

I tried my best to avoid churn in the JSON Swagger files, but the Azure.ResourceManager.CommonTypes package does not provide "update" models. So there is some unfortunate churn around the definitions for patching MSI and tags fields for tracked resource types.

PR Checklist

If E2E tests are included:

E2E tests follow Principles of Good E2E Test Case Design
If new E2E use case is covered (via a new test or new check/verifier),
demonstrate that the test is able to detect a defect/error and fail with
proper error message and logs which communicates nature of the problem.

openshift-ci · 2026-08-06T17:36:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mbarnes

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~api/OWNERS~~ [mbarnes]
~~internal/OWNERS~~ [mbarnes]
~~test/sdk/OWNERS~~ [mbarnes]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Ben Vesel (bennerv) · 2026-08-06T18:42:29Z

+   * Budgets that have not been successfully drained from a node will be forcibly evicted. This is
+   * especially relevant to cluster upgrades.
+   *
+   * Valid values are in minutes and from 0 to 10080 minutes (1 week).


nit:

Don't need "Valid values are in minutes and from 0 to 10080 minutes (1 week)." if we have a @max and @min decorator since they're rendered into the description i believe. Just call out the units.

Ben Vesel (bennerv) · 2026-08-06T18:43:01Z

-   *        of a TypeSpec bug with updatable fields. The default value will be
-   *        declared in a future API version once the TypeSpec bug is fixed.
-   *        https://github.com/Azure/typespec-azure/issues/1586
+   *  If not specified, the default value is 'stable'.


remove what the default is here. Also an update we don't take a default, so remove this text entirely.

Explicitly specify the version so local copies stay synchronized with what the CI check uses.

…tions Use // comment style in the TypeSpec files for annotations that we don't want published in customer-facing documentation. Things like workarounds for upstream TypeSpec bugs, etc.

Based on a buried blurb in a TypeSpec How-To [1]: Note: The ArmResourcePatch* templates are not recommended. They rely on Lifecycle.Update visibility analysis to automatically determine which properties are included in the PATCH schema, but this analysis is only performed by the typespec-autorest emitter and will not be replicated in SDKs generated for the PATCH operation. Instead, spec authors should define a specific PATCH model and use the ArmCustomPatch* templates. This means we have to revert back to maintaining separate update models for our resource types. :( [1] https://azure.github.io/typespec-azure/docs/howtos/arm/resource-operations/#resource-update-operations-patch

Regenerating SDKs for all API versions to verify no E2E breakage.

Ben Vesel (bennerv) · 2026-08-06T18:43:23Z

@@ -394,37 +508,22 @@ model ClusterAutoscalingProfile {

  /** maxPodGracePeriod is the maximum seconds to wait for graceful pod termination before scaling down a NodePool.
   * The default is 600 seconds.


nit: remove "The default is 600 seconds".

Ben Vesel (bennerv) · 2026-08-06T18:43:43Z

+  maxPodGracePeriodSeconds?: int32 = 600;

  /** maxNodeProvisionTimeSeconds is the maximum time to wait for node provisioning before considering the
   *  provisioning to be unsuccessful. The default is 900 seconds, or 15 minutes.


nit: remove "The default is 900 seconds, or 15 minutes."

Ben Vesel (bennerv) · 2026-08-06T18:43:58Z

+  maxNodeProvisionTimeSeconds?: int32 = 900;

  /** podPriorityThreshold enables users to schedule "best-effort" pods, which shouldn't trigger autoscaler actions,
   * but only run when there are spare resources available. The default is -10.


nit, remove "default" text.

Ben Vesel (bennerv) · 2026-08-06T18:44:39Z

+  /**
+   * The number of worker nodes, it cannot be used together with autoscaling.
+   * Validation:
+   * - Minimum: 0


remove min value specified in validation, it's in the decorator. Keep the max.

Ben Vesel (bennerv) · 2026-08-06T18:45:11Z

+   * If unset the cluster nodeDrainTimeoutMinutes value is used as a default.
+   */
+  @visibility(Lifecycle.Read, Lifecycle.Create, Lifecycle.Update)
+  nodeDrainTimeoutMinutes?: int32;


nit: add a Min Kim (@min) Max Schoening (@max) decorator, and remove valid values.

Ben Vesel (bennerv) · 2026-08-06T18:45:42Z

+   *  If not specified, the default value is 'stable'.
   */
  @visibility(Lifecycle.Read, Lifecycle.Create, Lifecycle.Update)
  channelGroup?: string;


"the default value is 'stable'" -> actually configure a default and set it. Then remove the default value text.

Ben Vesel (bennerv) · 2026-08-06T18:46:02Z

+   * provider, must be issued to. At least one of the entries must match the
+   * 'aud' claim in the JWT token.
+   *
+   * audiences must contain at least one entry and must not exceed ten entries.


not needed, it's part of min/max items.

Ben Vesel (bennerv) · 2026-08-06T18:46:38Z

+   *    - "username": the mapped value will be "https://myoidc.tld#userA"
+   *    - "email": the mapped value will be "userA@myoidc.tld"
   */
  prefixPolicy?: UsernameClaimPrefixPolicy;


remove default text in here. It should have the default defined in the typespec base one.

Ben Vesel (bennerv)

So it looks like the default doesn't get rendered into the descriptions on https://learn.microsoft.com/en-us/rest/api. But the min/max values do. So keep the default text on creates, but min/max values are rendered so we can omit them from descriptions afaict.

Worst case we add them back in.

Since we're not using the ArmResourcePatchAsync template anymore, Azure/typespec-azure#1586 is no longer relevant for us. We can drop the workarounds.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

openshift-ci · 2026-08-08T09:46:23Z

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Copilot AI lite review requested due to automatic review settings August 6, 2026 17:36

openshift-ci Bot requested review from Ben Vesel (bennerv) and Miguel Soriano (miguelsorianod) August 6, 2026 17:36

Matthew Barnes (mbarnes) removed request for Copilot and Miguel Soriano (miguelsorianod) August 6, 2026 17:36

openshift-ci Bot added the approved label Aug 6, 2026

Matthew Barnes (mbarnes) force-pushed the mbarnes/public-preview-typespec-rework branch from caff07b to c156e30 Compare August 6, 2026 17:51

Copilot AI lite review requested due to automatic review settings August 6, 2026 17:51

Copilot AI reviewed Aug 6, 2026

Copilot AI review requested due to automatic review settings August 6, 2026 18:09

Copilot AI reviewed Aug 6, 2026

Ben Vesel (bennerv) reviewed Aug 6, 2026

View reviewed changes

Matthew Barnes added 4 commits August 6, 2026 14:43

chore(api): Add @autorest/core to package.json

35173ba

Explicitly specify the version so local copies stay synchronized with what the CI check uses.

refactor(api): Use a different comment style for some TypeSpec annota…

1f56867

…tions Use // comment style in the TypeSpec files for annotations that we don't want published in customer-facing documentation. Things like workarounds for upstream TypeSpec bugs, etc.

chore(test): Regenerate all test SDKs with refactored TypeSpec

25db3e0

Regenerating SDKs for all API versions to verify no E2E breakage.

Ben Vesel (bennerv) reviewed Aug 6, 2026

View reviewed changes

refactor(api): Drop workaround for patch defaults

b7aabbb

Since we're not using the ArmResourcePatchAsync template anymore, Azure/typespec-azure#1586 is no longer relevant for us. We can drop the workarounds.

Copilot AI review requested due to automatic review settings August 6, 2026 19:56

Matthew Barnes (mbarnes) force-pushed the mbarnes/public-preview-typespec-rework branch from 3b68f27 to b7aabbb Compare August 6, 2026 19:56

Copilot AI reviewed Aug 6, 2026

Copilot started reviewing on behalf of Matthew Barnes (mbarnes) August 6, 2026 22:52 View session

Copilot started reviewing on behalf of Matthew Barnes (mbarnes) August 6, 2026 22:55 View session

Copilot started reviewing on behalf of Matthew Barnes (mbarnes) August 6, 2026 23:08 View session

Copilot started reviewing on behalf of Matthew Barnes (mbarnes) August 6, 2026 23:24 View session

openshift-ci Bot added the needs-rebase label Aug 8, 2026

		@@ -394,37 +508,22 @@ model ClusterAutoscalingProfile {

		/** maxPodGracePeriod is the maximum seconds to wait for graceful pod termination before scaling down a NodePool.
		* The default is 600 seconds.

Uh oh!

Conversation

Matthew Barnes (mbarnes) commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Testing

Special notes for your reviewer

PR Checklist

Uh oh!

openshift-ci Bot commented Aug 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI 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!

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!

Ben Vesel (bennerv) Aug 6, 2026 • 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!

Ben Vesel (bennerv) left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci Bot commented Aug 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Matthew Barnes (mbarnes) commented Aug 6, 2026 •

edited

Loading

Ben Vesel (bennerv) Aug 6, 2026 •

edited

Loading