hashicorp / pandora

A suite of single-purpose tools enabling automation for Terraform/Azure
Mozilla Public License 2.0
66 stars 48 forks source link

importer: importer-rest-api-specs cannot be successfully run on Windows platform #699

Open ms-zhenhua opened 2 years ago

ms-zhenhua commented 2 years ago

I found 2 problems:

  1. filepath.Join will genereate path like ....\swagger\specification\a\b in Windows platform. But FindResourceManagerServices in tools\importer-rest-api-specs\parser\discovery.go uses / to trim and split path. It will cause load data failure.
  2. After replacing / with filepath.Separator, services HealthcareApis and RedisEnterprise still cannot be imported successfully.
❌ Service "HealthcareApis" - Api Version "2021-06-01-preview"
2022/03/22 09:13:30      💥 Error: parsing Swagger files: parsing files in "..\\..\\swagger\\specification\\healthcareapis\\resource-manager\\Microsoft.HealthcareApis\\preview\\2021-06-01-preview": parsing file "\\healthcare-apis.json": flattening swagger file with references "..\\..\\swagger\\specification\\healthcareapis\\resource-manager\\Microsoft.HealthcareApis\\preview\\2021-06-01-preview\\healthcare-apis.json": at #/definitions/privateEndpointConnection/allOf/0, object has no key "Resource"
2022/03/22 09:13:30 importing Service "HealthcareApis" / Version "2021-06-01-preview": parsing Swagger files: parsing files in "..\\..\\swagger\\specification\\healthcareapis\\resource-manager\\Microsoft.HealthcareApis\\preview\\2021-06-01-preview": parsing file "\\healthcare-apis.json": flattening swagger file with references "..\\..\\swagger\\specification\\healthcareapis\\resource-manager\\Microsoft.HealthcareApis\\preview\\2021-06-01-preview\\healthcare-apis.json": at #/definitions/privateEndpointConnection/allOf/0, object has no key "Resource"

❌ Service "RedisEnterprise" - Api Version "2022-01-01"
2022/03/22 09:13:34      💥 Error: parsing Swagger files: parsing files in "..\\..\\swagger\\specification\\redisenterprise\\resource-manager\\Microsoft.Cache\\stable\\2022-01-01": parsing file "\\redisenterprise.json": flattening swagger file with references "..\\..\\swagger\\specification\\redisenterprise\\resource-manager\\Microsoft.Cache\\stable\\2022-01-01\\redisenterprise.json": at #/definitions/privateLinkResource/allOf/0, object has no key "Resource"
2022/03/22 09:13:34 importing Service "RedisEnterprise" / Version "2022-01-01": parsing Swagger files: parsing files in "..\\..\\swagger\\specification\\redisenterprise\\resource-manager\\Microsoft.Cache\\stable\\2022-01-01": parsing file "\\redisenterprise.json": flattening swagger file with references "..\\..\\swagger\\specification\\redisenterprise\\resource-manager\\Microsoft.Cache\\stable\\2022-01-01\\redisenterprise.json": at #/definitions/privateLinkResource/allOf/0, object has no key "Resource"
2022/03/22 09:13:35 ❌ Service "RedisEnterprise" - Api Version "2021-08-01"
2022/03/22 09:13:35      💥 Error: parsing Swagger files: parsing files in "..\\..\\swagger\\specification\\redisenterprise\\resource-manager\\Microsoft.Cache\\stable\\2021-08-01": parsing file "\\redisenterprise.json": flattening swagger file with references "..\\..\\swagger\\specification\\redisenterprise\\resource-manager\\Microsoft.Cache\\stable\\2021-08-01\\redisenterprise.json": at #/definitions/privateEndpointConnection/allOf/0, object has no key "Resource"
tombuildsstuff commented 2 years ago

@ms-zhenhua those are known issues - when importing a service into Pandora there should be a PR sent to onboard the service: https://github.com/hashicorp/pandora/blob/main/docs/generating-a-go-sdk.md#step-1-importing-swagger-data-into-pandoras-data-format

Once that PR is merged, it'll be imported - it's worth noting that if there isn't a Pandora PR for a service then we can't merge it into the Terraform Provider, so that PR is important per the documentation above.

Feel free to send a PR to fix the first issue you've mentioned above, the second can be fixed by setting the Environment Variable defined here: https://github.com/hashicorp/pandora/blob/49be57d6d4ad1b03e6551a24127ad47c6c89890f/tools/importer-rest-api-specs/main.go#L26 which unfortunately Windows doesn't pickup at runtime like other platforms - as such this should be fixed locally with both of these changes, but please send a PR for any Data imports so that we can ensure they're tracked too.

ms-zhenhua commented 2 years ago

Hi @tombuildsstuff, thank you for your quick reply. For the second problem, I found OAIGEN_DEDUPE is used in the uniqifyName method of github.com\jackofallops\analysis@v0.20.2-0.20210705135157-888aa8dbc8e5\flatten_name.go. I tried to set OAIGEN_DEDUPE to both true and false, but neither works and I still got the same error.

BTW, I found the error is returned by the following code snippet in getSingleImpl method of github.com\go-openapi\jsonpointer@v0.19.5\pointer.go. The decodedToken is Resource, but the map only have key value of resource. I also tried it on Ubuntu and found that the decodedToken on Ubuntu is resource so that Linux does not have such problem. But I do not know what cause the difference.

image

tombuildsstuff commented 2 years ago

@ms-zhenhua for the moment this can be run in CI (as that's where it's intended to be run) - this isn't intended to be run locally.

Whilst we may look to support this locally on Windows at some point (and we'd accept a PR to fix this if you can dig further into it) - as the Importer (and SDK Generator) are primarily intended to be run in CI once a PR is merged - which (in the not-too-distant future) will generate this into a Go SDK repository (and we'll stop embedding the Go SDK's within the Provider) - so as mentioned above in the documentation, please send a pull request for any new services which need to be supported in Pandora.

Thanks!

tombuildsstuff commented 1 year ago

@ms-zhenhua out of interest, is this one still an issue?

ms-zhenhua commented 1 year ago

Hi @tombuildsstuff, I haven't tried it with the latest version yet. Will try it later. If it is resolved, I will close this issue.

ms-zhenhua commented 1 year ago

Hi @tombuildsstuff, this issue still exists. I have created this PR to fix it.

jackofallops commented 1 year ago

Hi @ms-zhenhua - thanks for the above PR, but this is hard coding the path separator where we should be letting the OS decide? Are you be running the code on WSL?

ms-zhenhua commented 1 year ago

Hi @jackofallops, thank you for review. The problem happens in native Windows only. The reason is that path/filepath package is OS aware and always generate \\ separator in Windows. However, the generated string will be used by url.Parse. Since url.Parse hard codes / as the separator, the parsed result is different from that in Linux/MacOS. Based on this reason, I think it make sense to hard code / as the separator. WDYT?

For WSL, since it's a Linux environment, there is no problem.