ShiftLeft pipeline support request by Fuming Zhang: Sync Swagger to GitHub task is producing an empty diff

[konrad-jamrozik]

Relevant post on API Service Toolset Teams channel.

I had calls with Ray Chen about it. Recordings available on https://www.microsoft365.com/mycontent

Recording 1 - about ShiftLeft in general
Recording 2 - debugging this issue specifically

Urgency:

[Tuesday 3/14/2023 7:55 PM] Fuming Zhang
got it, it takes 6 days before service is ready to perform swagger release, so we could wait a little bit

Tasks that didn't diff anything:

• Sync TSP to GitHub - this is expected, see this comment below,
• Sync Swagger to GitHub - this is unexpected, and hence the problem.

[FumingZhang]

Hey @konrad-jamrozik, may I ask how is the progress of this bug fix? We want to do swagger release as soon as possible.

[konrad-jamrozik]

@FumingZhang apologizes for the delay! I am going to work on this over the next 12 hours, expect an update from me in that time window.

[konrad-jamrozik]

I have advised @FumingZhang to create a PR directly to the azure-rest-api-specs repo. This is in violation of the ShiftLeft rules, as can be seen e.g. here: Azure/azure-rest-api-specs#22309 (comment) .

However, the rule assumes timely support if issues like this one arise, which is currently not possible, as I am new to the codebase and I am still working through it, making progress, albeit slow. Fuming confirmed he is working on submitting a PR to the main repo.

[FumingZhang]

Manually created PR #23189.

[konrad-jamrozik]

I submitted a PR against ShiftLeft source adding diagnostic info to help diagnose the root-cause:
https://devdiv.visualstudio.com/DevDiv/_git/openapi-pipeline/pullrequest/459818

[konrad-jamrozik]

@FumingZhang I believe I found the root cause. In our call we had few days ago you gave as an example that the following directory should have been included in the diff PR:

/swagger/containerservice/resource-manager/Microsoft.ContainerService/aks/stable/2023-02-01

I ran another build from that branch with additional diagnostic output which you find here. In this build you can see:

CaldDirForArm='swagger/containerservice/fleets.management' CadlDirForDataplane=''
Executing Apply-PatchTsp src='/home/vsts/work/1/s/../Azure-azure-rest-api-specs-pr/swagger/containerservice/fleets.management' tar='specification' ServiceName='containerservice' tspFolder='fleets.management' targetTspFolder='specification/containerservice/fleets.management'
Executing Remove-DeletedTsp src='/home/vsts/work/1/s/../Azure-azure-rest-api-specs-pr/swagger/containerservice/fleets.management' tar='specification/containerservice/fleets.management'
removing whole folder specification/containerservice/fleets.management
Skipped processing CadlDirForDataplane as it is an empty string
Executing Push-Changes branch='containerservice/official/aks-api-release' commitMsg='[AutoSyncTsp] bae8054ad7e Merged PR 7768257: Revert "Merged PR 7726419: [API Release] 2023-03-02-preview Create Swagger ba...'
No tsp changes

Please note the:
CaldDirForArm='swagger/containerservice/fleets.management'

this is a different path than the example you gave, of:

/swagger/containerservice/resource-manager/Microsoft.ContainerService/aks/stable/2023-02-01

it differs by suffix: fleets.management vs resource-manager.

Hence it seems to me that if you would change the value of the CadlDirForArm variable, which currently is swagger/containerservice/fleets.management, the PR should be properly created.

Now all future builds using ShiftLeft should have this diagnostic information, thus making it easier to figure out what went wrong.

Let me know if this solves the issue, thanks!

[FumingZhang]

@konrad-jamrozik, there are 2 sub-services under the Microsoft.ContainerService namespace, the first one is aks (with path /swagger/containerservice/resource-manager/Microsoft.ContainerService/aks) developed in the traditional way (modify the json file directly), and the second one is fleet (with path /swagger/containerservice/resource-manager/Microsoft.ContainerService/fleet) developed with cadl.

The API changes of these two services will be released independently, so we defined the manifest file like

"official/aks-api-release": {
    "includedPaths": [
        "containerservice/resource-manager/Microsoft.ContainerService/aks/*"
    ]
},
"official/fleet-api-release": {
    "includedPaths": [
        "containerservice/resource-manager/Microsoft.ContainerService/fleet/*"
    ]
}

This time, I am trying to release aks related changes via branch official/aks-api-release and I think it should have nothing to do with fleet or cadl (not sure what Tsp is short for, but I guess it's related to cadl).

So once we've enabled cadl, it's effective for the whole namespace? Can the traditional way and cadl way coexist?

[konrad-jamrozik]

@FumingZhang apologies, my mistake. I somehow assumed the issue is that both Sync Swagger to GitHub and Sync Tsp to GitHub do not create any diff, but obviously you meant only Sync Swagger to GitHub. Hence, I will need to work a bit more on getting the root cause, but relevant code to which I need to add diagnostic logging is very similar, so expect a reply from me within next 24h (i.e. my upcoming business day).

Can the traditional way and cadl way coexist?

Yes, they can co-exist. There are two separate tasks for that in the ShiftLeft pipeline.

I believe Tsp stands for TypeSpec which is the new name for Cadl.

[konrad-jamrozik]

@FumingZhang I submitted a PR with experimental bugfix, waiting for approval:
Pull Request 460255: Path matching bugfixes in SyncTspToGitHub.ps1 & SyncSwaggerToGitHub.ps1 for backslashes, and for case of non-existent excludedPaths.

[FumingZhang]

Thanks for the update

[konrad-jamrozik]

@FumingZhang I fixed the immediate bug with this PR, but I uncovered the next bug; currently waiting for approval for a PR with a fix.

[konrad-jamrozik]

@FumingZhang OK looks like the bug is finally fixed!

PR with the last bugfix

Successful build

Resulting PR, confirming the fix works

Now I am going to look into #5788

I am closing this issue as resolved, but let me know if something is still not right.

[FumingZhang]

Cool, thanks @konrad-jamrozik.