[Avocado] Allow specs with `readme.md` in sub-folders under `resource-manager`

[mikeharder]

PR https://github.com/Azure/azure-rest-api-specs-pr/pull/21133 adds a readme.md like this:

specification/foo/resource-manager/Private.Foo/bar/readme.md

It does not have a readme.md directly under resource-manager. This causes check Avocado to fail:

{
  "level":"Error",
  "code":"MISSING_README",
  "message":"Can not find readme.md in the folder. If no readme.md file, it will block SDK generation.",
  "path":"/mnt/vss/_work/1/sha/specification/foo/resource-manager",
  "folderUrl":"/mnt/vss/_work/1/sha/specification/foo/resource-manager"
}
avocadoForDir: /mnt/vss/_work/1/sha/specification/foo/resource-manager/Private.Foo/bar
errors: 1

https://github.com/Azure/azure-rest-api-specs-pr/pull/21133/checks?check_run_id=36616007878

Should this cause Avocado to fail, or is a bug in the check?

Other checks like LintDiff appear to find the readme.md correctly:

RETURN definition getChangingConfigureFiles
all affected readme are:
[
  "specification/foo/resource-manager/Private.Foo/bar/readme.md"
]

https://dev.azure.com/azure-sdk/internal/_build/results?buildId=4529937&view=logs&j=0574a2a6-2d0a-5ec6-40e4-4c6e2f70bea2&t=80c3e782-49f0-5d1c-70dd-cbee57bdd0c7&l=61

CC: @weshaggard, @JeffreyRichter

[JeffreyRichter]

With directory, "specification/foo/resource-manager/Private.Foo/bar/readme.md", Private.Foo is the RP namespace and bar is a service name. The service should have a readme.md in it.

I know that there are readme.md files directly under the RP namespace dir and I want to abolish this going forward.

[mikeharder]

With directory, "specification/foo/resource-manager/Private.Foo/bar/readme.md", Private.Foo is the RP namespace and bar is a service name. The service should have a readme.md in it.

I know that there are readme.md files directly under the RP namespace dir and I want to abolish this going forward.

I think the question is slightly different. We are discussing 3 files:

1. specification/foo/resource-manager/readme.md
2. specification/foo/resource-manager/Private.Foo/readme.md
3. specification/foo/resource-manager/Private.Foo/bar/readme.md

The PR in question (https://github.com/Azure/azure-rest-api-specs-pr/pull/21133) adds only 3, and neither 1 nor 2. I haven't checked in detail, but the PR appears to work correctly with our checks, except for Avocado.

I believe you are suggesting disallowing 2, which may be a good idea, but this isn't relevant to my question.

Check Avocado (which is currently optional, but we want to make required again) requires 1. Should it? Currently, all specs contain file resource-manager/readme.md except these five:

specification/dynamicstelemetry/resource-manager
specification/azsadmin/resource-manager
specification/containerservice/resource-manager
specification/edge/resource-manager
specification/redhatopenshift/resource-manager

[JeffreyRichter]

I don't think #1 this should be allowed at all: specification/foo/resource-manager/readme.md even today.
Years ago, someone must have thought this was a good idea, but it isn't as it's not clear what it means.
What it mut mean is that the RP namespace (further down in the path) contains just 1 service and this is the readme.md for it. But logically, this doesn't make sense as the readme.md isn't tied to anything and if the RP namespace ever wanted to add another service, the directory structure & its contents would have to change.

I am suggestion disallowing 1 & 2 in the fullness of time and #3 is the only valid. I would suggest forcing #3 only now for new PRs to make transitioning easier in the future.

[mikeharder]

I don't think we are ready to disallow anything previously allowed. This would need to happen as part of a larger folder restructuring, with a comprehensive plan for what moves when, what applies to only new vs new and old, etc.

For now, it sounds like Avocado needs to change, to allow 1 and/or 3.

@rkmanda: Do you agree from the ARM side?

[JeffreyRichter]

Yes, allow 1 and 3 for now.
I did just learn more: The readme.md file directly under resource-manager folder allowed an SDK to be created from endpoints under multiple RP namespaces. But the ARM team (and doc & SDK teams) want to disallow this scenario entirely. So, if there are any readme.md files over 2+ RP namespace folders, I (and the ARM team) will have to work with these teams to fix this. I don't think there are many of these and it doesn't affect the deployed RPs/services, just SDK packages.