From 628f223b2b4ac4b6e4a27188c6ce661b98b24be9 Mon Sep 17 00:00:00 2001 From: Gerry Tan Date: Thu, 24 Oct 2024 14:51:45 +1100 Subject: [PATCH 1/6] Various improvements from feedback from .NET SDK review --- .../client.tsp | 8 ++++++++ .../models.tsp | 18 +++++++++-------- .../routes.tsp | 2 +- .../preview/2023-07-01-preview/export.json | 20 +++++++++++-------- 4 files changed, 31 insertions(+), 17 deletions(-) diff --git a/specification/terraform/Microsoft.AzureTerraform.Management/client.tsp b/specification/terraform/Microsoft.AzureTerraform.Management/client.tsp index ccb51a59817e..4989342a215c 100644 --- a/specification/terraform/Microsoft.AzureTerraform.Management/client.tsp +++ b/specification/terraform/Microsoft.AzureTerraform.Management/client.tsp @@ -2,6 +2,14 @@ import "@azure-tools/typespec-client-generator-core"; import "./main.tsp"; using Azure.ClientGenerator.Core; +using Azure.ResourceManager; using Microsoft.AzureTerraform; +@@clientName(ExportResult, "TerraformExportResult", "csharp"); +@@clientName(ResourceProvisioningState, + "TerraformResourceProvisioningState", + "csharp" +); +@@clientName(TargetTerraformProvider.azapi, "AzApi", "csharp"); +@@clientName(TargetTerraformProvider.azurerm, "AzureRM", "csharp"); @@clientName(Terraform.exportTerraform, "ExportTerraform", "csharp"); diff --git a/specification/terraform/Microsoft.AzureTerraform.Management/models.tsp b/specification/terraform/Microsoft.AzureTerraform.Management/models.tsp index 2f471a013c3f..6191bdb0d0cb 100644 --- a/specification/terraform/Microsoft.AzureTerraform.Management/models.tsp +++ b/specification/terraform/Microsoft.AzureTerraform.Management/models.tsp @@ -1,9 +1,11 @@ import "@typespec/rest"; import "@typespec/http"; +import "@azure-tools/typespec-azure-core"; import "@azure-tools/typespec-azure-resource-manager"; using TypeSpec.Rest; using TypeSpec.Http; +using Azure.Core; using Azure.ResourceManager; using Azure.ResourceManager.Foundations; using OpenAPI; @@ -21,7 +23,7 @@ union Type { } @doc("The target Azure Terraform Provider") -union targetProvider { +union TargetTerraformProvider { string, @doc("https://registry.terraform.io/providers/hashicorp/azurerm/latest") @@ -33,12 +35,12 @@ union targetProvider { @doc("The base export parameter") @discriminator("type") -model BaseExportModel { +model CommonExportProperties { @doc("The parameter type") type: Type; @doc("The target Azure Terraform Provider") - targetProvider?: targetProvider = targetProvider.azurerm; + targetProvider?: TargetTerraformProvider = TargetTerraformProvider.azurerm; @doc("Whether to output all non-computed properties in the generated Terraform configuration? This probably needs manual modifications to make it valid") fullProperties?: boolean = true; @@ -48,7 +50,7 @@ model BaseExportModel { } @doc("Export parameter for resources queried by ARG (Azure Resource Graph)") -model ExportQuery extends BaseExportModel { +model ExportQuery extends CommonExportProperties { @doc("The ARG where predicate. Note that you can combine multiple conditions in one `where` predicate, e.g. `resourceGroup =~ \"my-rg\" and type =~ \"microsoft.network/virtualnetworks\"`") query: string; @@ -63,9 +65,9 @@ model ExportQuery extends BaseExportModel { } @doc("Export parameter for individual resources.") -model ExportResource extends BaseExportModel { +model ExportResource extends CommonExportProperties { @doc("The id of the resource to be exported") - resourceIds: string[]; + resourceIds: armResourceIdentifier[]; @doc("The Terraform resource name. Only works when `resourceIds` contains only one item.") resourceName?: string = "res-0"; @@ -81,7 +83,7 @@ model ExportResource extends BaseExportModel { } @doc("Export parameter for a resource group") -model ExportResourceGroup extends BaseExportModel { +model ExportResourceGroup extends CommonExportProperties { @doc("The name of the resource group to be exported") resourceGroupName: string; @@ -101,7 +103,7 @@ model ExportResult { configuration?: string; @doc("A list of Azure resources which are not exported to Terraform due to there is no corresponding resources in Terraform") - skippedResources?: string[]; + skippedResources?: armResourceIdentifier[]; @doc("A list of errors derived during exporting each resource") @extension("x-ms-identifiers", []) diff --git a/specification/terraform/Microsoft.AzureTerraform.Management/routes.tsp b/specification/terraform/Microsoft.AzureTerraform.Management/routes.tsp index b989c729e631..07233090fe6d 100644 --- a/specification/terraform/Microsoft.AzureTerraform.Management/routes.tsp +++ b/specification/terraform/Microsoft.AzureTerraform.Management/routes.tsp @@ -23,7 +23,7 @@ interface Terraform { @tag("ExportTerraform") @useFinalStateVia("azure-async-operation") exportTerraform is ArmProviderActionAsync< - BaseExportModel, + CommonExportProperties, ArmAcceptedLroResponse<"Resource operation accepted.", LroHeaders>, Scope = SubscriptionActionScope, LroHeaders = LroHeaders diff --git a/specification/terraform/resource-manager/Microsoft.AzureTerraform/preview/2023-07-01-preview/export.json b/specification/terraform/resource-manager/Microsoft.AzureTerraform/preview/2023-07-01-preview/export.json index 59f4192cb53b..3b7c2d8e9831 100644 --- a/specification/terraform/resource-manager/Microsoft.AzureTerraform/preview/2023-07-01-preview/export.json +++ b/specification/terraform/resource-manager/Microsoft.AzureTerraform/preview/2023-07-01-preview/export.json @@ -107,7 +107,7 @@ "description": "The request body", "required": true, "schema": { - "$ref": "#/definitions/BaseExportModel" + "$ref": "#/definitions/CommonExportProperties" } } ], @@ -187,7 +187,7 @@ }, "readOnly": true }, - "BaseExportModel": { + "CommonExportProperties": { "type": "object", "description": "The base export parameter", "properties": { @@ -204,7 +204,7 @@ "azapi" ], "x-ms-enum": { - "name": "targetProvider", + "name": "TargetTerraformProvider", "modelAsString": true, "values": [ { @@ -260,7 +260,7 @@ ], "allOf": [ { - "$ref": "#/definitions/BaseExportModel" + "$ref": "#/definitions/CommonExportProperties" } ], "x-ms-discriminator-value": "ExportQuery" @@ -273,7 +273,9 @@ "type": "array", "description": "The id of the resource to be exported", "items": { - "type": "string" + "type": "string", + "format": "arm-id", + "description": "A type definition that refers the id to an Azure Resource Manager resource." } }, "resourceName": { @@ -296,7 +298,7 @@ ], "allOf": [ { - "$ref": "#/definitions/BaseExportModel" + "$ref": "#/definitions/CommonExportProperties" } ], "x-ms-discriminator-value": "ExportResource" @@ -320,7 +322,7 @@ ], "allOf": [ { - "$ref": "#/definitions/BaseExportModel" + "$ref": "#/definitions/CommonExportProperties" } ], "x-ms-discriminator-value": "ExportResourceGroup" @@ -337,7 +339,9 @@ "type": "array", "description": "A list of Azure resources which are not exported to Terraform due to there is no corresponding resources in Terraform", "items": { - "type": "string" + "type": "string", + "format": "arm-id", + "description": "A type definition that refers the id to an Azure Resource Manager resource." } }, "errors": { From 5aac6adbe1f38d76e5c98ae3dc3e8af0617f9dcc Mon Sep 17 00:00:00 2001 From: Gerry Tan Date: Thu, 24 Oct 2024 15:46:36 +1100 Subject: [PATCH 2/6] @@clientName for ExportResult.skippedResources --- .../terraform/Microsoft.AzureTerraform.Management/client.tsp | 1 + 1 file changed, 1 insertion(+) diff --git a/specification/terraform/Microsoft.AzureTerraform.Management/client.tsp b/specification/terraform/Microsoft.AzureTerraform.Management/client.tsp index 4989342a215c..bd8f1a5a5096 100644 --- a/specification/terraform/Microsoft.AzureTerraform.Management/client.tsp +++ b/specification/terraform/Microsoft.AzureTerraform.Management/client.tsp @@ -6,6 +6,7 @@ using Azure.ResourceManager; using Microsoft.AzureTerraform; @@clientName(ExportResult, "TerraformExportResult", "csharp"); +@@clientName(ExportResult.skippedResources, "SkippedResourceIds", "csharp"); @@clientName(ResourceProvisioningState, "TerraformResourceProvisioningState", "csharp" From 41aa43408d1164cc953e45d29e9b138433dcdc3a Mon Sep 17 00:00:00 2001 From: Gerry Tan Date: Thu, 24 Oct 2024 17:03:14 +1100 Subject: [PATCH 3/6] Revert spec model change, implement as client.tsp override instead --- .../Microsoft.AzureTerraform.Management/client.tsp | 6 ++++-- .../Microsoft.AzureTerraform.Management/models.tsp | 12 ++++++------ .../Microsoft.AzureTerraform.Management/routes.tsp | 2 +- .../preview/2023-07-01-preview/export.json | 12 ++++++------ 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/specification/terraform/Microsoft.AzureTerraform.Management/client.tsp b/specification/terraform/Microsoft.AzureTerraform.Management/client.tsp index bd8f1a5a5096..531ef00f5dc3 100644 --- a/specification/terraform/Microsoft.AzureTerraform.Management/client.tsp +++ b/specification/terraform/Microsoft.AzureTerraform.Management/client.tsp @@ -5,12 +5,14 @@ using Azure.ClientGenerator.Core; using Azure.ResourceManager; using Microsoft.AzureTerraform; +@@clientName(BaseExportModel, "CommonExportProperties", "csharp"); @@clientName(ExportResult, "TerraformExportResult", "csharp"); @@clientName(ExportResult.skippedResources, "SkippedResourceIds", "csharp"); @@clientName(ResourceProvisioningState, "TerraformResourceProvisioningState", "csharp" ); -@@clientName(TargetTerraformProvider.azapi, "AzApi", "csharp"); -@@clientName(TargetTerraformProvider.azurerm, "AzureRM", "csharp"); +@@clientName(targetProvider, "TargetTerraformProvider", "csharp"); +@@clientName(targetProvider.azapi, "AzApi", "csharp"); +@@clientName(targetProvider.azurerm, "AzureRM", "csharp"); @@clientName(Terraform.exportTerraform, "ExportTerraform", "csharp"); diff --git a/specification/terraform/Microsoft.AzureTerraform.Management/models.tsp b/specification/terraform/Microsoft.AzureTerraform.Management/models.tsp index 6191bdb0d0cb..154a5b737b3c 100644 --- a/specification/terraform/Microsoft.AzureTerraform.Management/models.tsp +++ b/specification/terraform/Microsoft.AzureTerraform.Management/models.tsp @@ -23,7 +23,7 @@ union Type { } @doc("The target Azure Terraform Provider") -union TargetTerraformProvider { +union targetProvider { string, @doc("https://registry.terraform.io/providers/hashicorp/azurerm/latest") @@ -35,12 +35,12 @@ union TargetTerraformProvider { @doc("The base export parameter") @discriminator("type") -model CommonExportProperties { +model BaseExportModel { @doc("The parameter type") type: Type; @doc("The target Azure Terraform Provider") - targetProvider?: TargetTerraformProvider = TargetTerraformProvider.azurerm; + targetProvider?: targetProvider = targetProvider.azurerm; @doc("Whether to output all non-computed properties in the generated Terraform configuration? This probably needs manual modifications to make it valid") fullProperties?: boolean = true; @@ -50,7 +50,7 @@ model CommonExportProperties { } @doc("Export parameter for resources queried by ARG (Azure Resource Graph)") -model ExportQuery extends CommonExportProperties { +model ExportQuery extends BaseExportModel { @doc("The ARG where predicate. Note that you can combine multiple conditions in one `where` predicate, e.g. `resourceGroup =~ \"my-rg\" and type =~ \"microsoft.network/virtualnetworks\"`") query: string; @@ -65,7 +65,7 @@ model ExportQuery extends CommonExportProperties { } @doc("Export parameter for individual resources.") -model ExportResource extends CommonExportProperties { +model ExportResource extends BaseExportModel { @doc("The id of the resource to be exported") resourceIds: armResourceIdentifier[]; @@ -83,7 +83,7 @@ model ExportResource extends CommonExportProperties { } @doc("Export parameter for a resource group") -model ExportResourceGroup extends CommonExportProperties { +model ExportResourceGroup extends BaseExportModel { @doc("The name of the resource group to be exported") resourceGroupName: string; diff --git a/specification/terraform/Microsoft.AzureTerraform.Management/routes.tsp b/specification/terraform/Microsoft.AzureTerraform.Management/routes.tsp index 07233090fe6d..b989c729e631 100644 --- a/specification/terraform/Microsoft.AzureTerraform.Management/routes.tsp +++ b/specification/terraform/Microsoft.AzureTerraform.Management/routes.tsp @@ -23,7 +23,7 @@ interface Terraform { @tag("ExportTerraform") @useFinalStateVia("azure-async-operation") exportTerraform is ArmProviderActionAsync< - CommonExportProperties, + BaseExportModel, ArmAcceptedLroResponse<"Resource operation accepted.", LroHeaders>, Scope = SubscriptionActionScope, LroHeaders = LroHeaders diff --git a/specification/terraform/resource-manager/Microsoft.AzureTerraform/preview/2023-07-01-preview/export.json b/specification/terraform/resource-manager/Microsoft.AzureTerraform/preview/2023-07-01-preview/export.json index 3b7c2d8e9831..00a264c31dde 100644 --- a/specification/terraform/resource-manager/Microsoft.AzureTerraform/preview/2023-07-01-preview/export.json +++ b/specification/terraform/resource-manager/Microsoft.AzureTerraform/preview/2023-07-01-preview/export.json @@ -107,7 +107,7 @@ "description": "The request body", "required": true, "schema": { - "$ref": "#/definitions/CommonExportProperties" + "$ref": "#/definitions/BaseExportModel" } } ], @@ -187,7 +187,7 @@ }, "readOnly": true }, - "CommonExportProperties": { + "BaseExportModel": { "type": "object", "description": "The base export parameter", "properties": { @@ -204,7 +204,7 @@ "azapi" ], "x-ms-enum": { - "name": "TargetTerraformProvider", + "name": "targetProvider", "modelAsString": true, "values": [ { @@ -260,7 +260,7 @@ ], "allOf": [ { - "$ref": "#/definitions/CommonExportProperties" + "$ref": "#/definitions/BaseExportModel" } ], "x-ms-discriminator-value": "ExportQuery" @@ -298,7 +298,7 @@ ], "allOf": [ { - "$ref": "#/definitions/CommonExportProperties" + "$ref": "#/definitions/BaseExportModel" } ], "x-ms-discriminator-value": "ExportResource" @@ -322,7 +322,7 @@ ], "allOf": [ { - "$ref": "#/definitions/CommonExportProperties" + "$ref": "#/definitions/BaseExportModel" } ], "x-ms-discriminator-value": "ExportResourceGroup" From d16ae2617a752878c5030e45467f296e81974d3c Mon Sep 17 00:00:00 2001 From: Gerry Tan Date: Fri, 25 Oct 2024 09:12:44 +1100 Subject: [PATCH 4/6] Rename ExportResource in csharp --- .../terraform/Microsoft.AzureTerraform.Management/client.tsp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/specification/terraform/Microsoft.AzureTerraform.Management/client.tsp b/specification/terraform/Microsoft.AzureTerraform.Management/client.tsp index 531ef00f5dc3..d8ec7ce52474 100644 --- a/specification/terraform/Microsoft.AzureTerraform.Management/client.tsp +++ b/specification/terraform/Microsoft.AzureTerraform.Management/client.tsp @@ -6,6 +6,9 @@ using Azure.ResourceManager; using Microsoft.AzureTerraform; @@clientName(BaseExportModel, "CommonExportProperties", "csharp"); +// Name ending with "Resource" is reserved for ARM resource in .NET SDK +// Refer to https://github.com/Azure/azure-sdk-for-net/blob/4dacd22df0cf904b11cb3b1389aa566c552fdd0f/common/ManagementTestShared/Redesign/InheritanceCheckTests.cs#L23 +@@clientName(ExportResource, "ExportResourceTerraform", "csharp"); @@clientName(ExportResult, "TerraformExportResult", "csharp"); @@clientName(ExportResult.skippedResources, "SkippedResourceIds", "csharp"); @@clientName(ResourceProvisioningState, From 6843a6f8e11d51c109746a60356c16e405551072 Mon Sep 17 00:00:00 2001 From: Gerry Tan Date: Fri, 25 Oct 2024 12:43:39 +1100 Subject: [PATCH 5/6] Added Terraform suffix to other Export... model names --- .../terraform/Microsoft.AzureTerraform.Management/client.tsp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/specification/terraform/Microsoft.AzureTerraform.Management/client.tsp b/specification/terraform/Microsoft.AzureTerraform.Management/client.tsp index d8ec7ce52474..eb86d30dd659 100644 --- a/specification/terraform/Microsoft.AzureTerraform.Management/client.tsp +++ b/specification/terraform/Microsoft.AzureTerraform.Management/client.tsp @@ -8,7 +8,9 @@ using Microsoft.AzureTerraform; @@clientName(BaseExportModel, "CommonExportProperties", "csharp"); // Name ending with "Resource" is reserved for ARM resource in .NET SDK // Refer to https://github.com/Azure/azure-sdk-for-net/blob/4dacd22df0cf904b11cb3b1389aa566c552fdd0f/common/ManagementTestShared/Redesign/InheritanceCheckTests.cs#L23 +@@clientName(ExportQuery, "ExportQueryTerraform", "csharp"); @@clientName(ExportResource, "ExportResourceTerraform", "csharp"); +@@clientName(ExportResourceGroup, "ExportResourceGroupTerraform", "csharp"); @@clientName(ExportResult, "TerraformExportResult", "csharp"); @@clientName(ExportResult.skippedResources, "SkippedResourceIds", "csharp"); @@clientName(ResourceProvisioningState, From 0756ab6749f87d935a6a0c497523fa021a4a55e3 Mon Sep 17 00:00:00 2001 From: Arthur Ma Date: Mon, 28 Oct 2024 18:31:12 +0800 Subject: [PATCH 6/6] update --- .../Microsoft.AzureTerraform.Management/client.tsp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/specification/terraform/Microsoft.AzureTerraform.Management/client.tsp b/specification/terraform/Microsoft.AzureTerraform.Management/client.tsp index eb86d30dd659..ccee335ee281 100644 --- a/specification/terraform/Microsoft.AzureTerraform.Management/client.tsp +++ b/specification/terraform/Microsoft.AzureTerraform.Management/client.tsp @@ -5,10 +5,17 @@ using Azure.ClientGenerator.Core; using Azure.ResourceManager; using Microsoft.AzureTerraform; +@@clientName(Type, "CommonExportType", "csharp"); @@clientName(BaseExportModel, "CommonExportProperties", "csharp"); +@@clientName(BaseExportModel.fullProperties, + "IsOutputFullPropertiesEnabled", + "csharp" +); +@@clientName(BaseExportModel.maskSensitive, "IsMaskSensitiveEnabled", "csharp"); // Name ending with "Resource" is reserved for ARM resource in .NET SDK // Refer to https://github.com/Azure/azure-sdk-for-net/blob/4dacd22df0cf904b11cb3b1389aa566c552fdd0f/common/ManagementTestShared/Redesign/InheritanceCheckTests.cs#L23 @@clientName(ExportQuery, "ExportQueryTerraform", "csharp"); +@@clientName(ExportQuery.recursive, "IsRecursive", "csharp"); @@clientName(ExportResource, "ExportResourceTerraform", "csharp"); @@clientName(ExportResourceGroup, "ExportResourceGroupTerraform", "csharp"); @@clientName(ExportResult, "TerraformExportResult", "csharp");