[typespec-validation] Error should be reported while missing 'Management' suffix in the folder name of management plane typespec

[raych1]

Context Per the typespec folder structure requirement, it says "Management at the end of the service RP-name indicates a management (resource manager) library". The folder should be like the following example:

Additionally, SDK automation tool depends on the 'Management' suffix in the folder name to differentiate the management plane typespec and data plane typespec to call the proper code generator (MPG or DPG) to generate the SDK.

I found typespec-validation tool has the following code to check folder name for management plane typespec case. Request to extend it to gate on the suffix. https://github.com/Azure/azure-rest-api-specs/blob/d1296700aa6cd650970e9891dd58eef5698327fd/eng/tools/typespec-validation/src/rules/folder-structure.ts#L56

CC: @mikeharder @weshaggard @weidongxu-microsoft

[mikeharder]

@raych1: Can you please be more specific, and give some examples of specs that should pass and fail the new rule?

Are you just asking for the rule to ensure that, if a folder name contains "Management", that the folder ends with either "Management" or "Management.Shared"?

Or is it more complex, say based on the contents of the folder? Something like "If a *.tsp contains <foo>, then the folder name must end with Management"?

[weidongxu-microsoft]

Just an option:

• If service configure typespec-autorest to output to resource-manager folder (e.g. oracle), this tspconfig.yaml should be in a folder with suffix .Management

[mikeharder]

Just an option:

• If service configure typespec-autorest to output to resource-manager folder (e.g. oracle), this tspconfig.yaml should be in a folder with suffix .Management

This check could go on L84:

https://github.com/Azure/azure-rest-api-specs/blob/d1296700aa6cd650970e9891dd58eef5698327fd/eng/tools/typespec-validation/src/rules/folder-structure.ts#L77-L89

[raych1]

@mikeharder ,

I agree with @weidongxu-microsoft 's suggestion. I'm not sure if azure-resource-provider-folder is required field to set in tspconfig.yaml for typespec-autorest emitter. We might need to cover the cases if it's not set.

In case of the field not being set, we may verify if the spec PR contains any change to the resource-manager folder.

[mikeharder]

In case of the field not being set, we may verify if the spec PR contains any change to the resource-manager folder.

Should already be covered here:

https://github.com/Azure/azure-rest-api-specs/blob/d1296700aa6cd650970e9891dd58eef5698327fd/eng/tools/typespec-validation/src/rules/linter-ruleset.ts#L49-L67

[raych1]

When the rpFolder ends with resource-manager, the corresponding typespec folder should have a suffix of 'Management'.

[mikeharder]

Proposed fix here: https://github.com/Azure/azure-rest-api-specs/pull/29663

However, we won't be able to merge this, until we fix or suppress any existing violations (like oracle). Check "TypeSpecValidation-All" will let us know how many existing specs in main are failing.

[mikeharder]

6 specs are failing: https://dev.azure.com/azure-sdk/public/_build/results?buildId=3926861&view=results

@raych1, @weshaggard, @konrad-jamrozik: Do you think we should rename all these spec folders to end in .Management?

[konrad-jamrozik]

@mikeharder how about this:

1. add suppression file(s) for these 6 specs (I think we do support this already, yes?)
2. submit a PR to rename these dirs
3. get approval from the owners of the renamed specs

[raych1]

@mikeharder , I suggest renaming all the six services identified with incorrect suffix. This renaming will not need config change or impact SDK generation. //CC: @weidongxu-microsoft

In addition, for the RPaaS service RPs, it depends on the folder path to fetch the spec to do the API call live validation. Therefore, the renaming will require the RP registration update in RPaaS metaRP side. We might need individual PRs for the RpaaS service RP so that service team can determine the proper merge time.

[mikeharder]

@mikeharder how about this:

1. add suppression file(s) for these 6 specs (I think we do support this already, yes?)
2. submit a PR to rename these dirs
3. get approval from the owners of the renamed specs

I generally agree, however:

1. TSV currently only supports supressing all of TSV. You can't suppress individual rules (or in this case, part of a rule). So #29663 will be blocked by #24955, but I will prioritize both of these. This is a good forcing function.

2. It sounds like we should use one PR per spec. Since this is a simple rename and only 6 specs, and it was our miss for not checking this earlier, I'm fine creating these PRs myself. I can actually create these PRs in parallel with updating TSV.