fix(openapi2kong): removing unused component reference

[marckong]

This PR is to address the issue mentioned in this comment.

• added several recursive methods to achieve dereferencing only used ref paths in the given OpenAPI V3 schema object
• added unit test corrections in the affected areas
• added unit test case for circular dependency issue

I found it exhausting to work on the openapi2kong. I didn't have energy to really care about the code quality for the recursions written in this PR. If you find it can be enhanced, please feel free to commit the enhanced version of the recursions.

changelog(Improvements): Improved Kong Declarative Config generation for OpenAPI specs with circular dependencies and also added logic to ignore unreferenced components from the generated config

[marckong]

Please feel free to add a unit test for this or enhance.

[Tieske]

This looks good! a circular reference test, and getting rid of unused references. Especially the latter will really slim down the resulting configs.

I'll leave it to others to judge the code and test coverage.

[dimitropoulos]

we made some great progress simplifying the test case, but now we need to reevaluate based on what we learned (real quick!)

[filfreire]

LGTM! 👍

There's still one issue ⚠️

I'm getting an error on generated files, when running deck sync -s <file>:

(...)
Summary:
  Created: 0
  Updated: 1
  Deleted: 0
Error: 1 errors occurred:
        while processing event: {Create} plugin request-validator for route 3bc7df06-b6c3-4d21-a8db-8a7a3ec79b25 failed: HTTP status 400 (message: "schema violation ([string \"jsonschema:anonymous\"]:20: stack overflow)")

Note: deck validate -s <file> seems to work ok.

Sharing here the generated files in case it helps:
Archive.zip

[Tieske]

just ried importing packages/openapi-2-kong/src/declarative-config/fixtures/circular-requestBody.expected.json directly into Kong (skipping decK), and it fails as well;

[Kong-E-2.8.1.0:inso:/kong]$ kong config db_import /kong-plugin/kong.json
Error: Failed parsing:
in 'services':
  - in entry 1 of 'services':
    in 'routes':
      - in entry 1 of 'routes':
        in 'plugins':
          - in entry 1 of 'plugins':
            in '@entity':
              - in entry 1 of '@entity': [string "jsonschema:anonymous"]:9: stack overflow

  Run with --v (verbose) or --vv (debug) for more details
[Kong-E-2.8.1.0:inso:/kong]$

Looks like the problem is with Kong, or even the JSONschema library used 😞 . I thought I had checked this, but apparently I missed something. Maybe I checked the lib, but not Kong 🤔

[marckong]

Hi @Tieske , can you please try it with the actual example instead of circular-requestBody.expected.json

[marckong]

The circular-requestBody.expected.json was introduced to narrow down to minimal requirements to reproduce the bug but that may not reflect on the real schema conversions. For instance, "post" path has request but no response defined. I am still curious what makes such difference, but when I ran with the following configuration generated, it passes db_import command.

{
	"_format_version": "1.1",
	"services": [
		{
			"name": "Testing_Circular_3",
			"protocol": "https",
			"host": "some.random.url",
			"port": 443,
			"path": "/",
			"plugins": [
				{
					"name": "request-validator",
					"config": {
						"version": "draft4",
						"body_schema": "{}",
						"verbose_response": true
					},
					"tags": [
						"OAS3_import"
					],
					"enabled": true
				}
			],
			"routes": [
				{
					"tags": [
						"OAS3_import"
					],
					"name": "Testing_Circular_3-testing-testingId-testing-endpoint-post",
					"methods": [
						"POST"
					],
					"paths": [
						"/testing/(?<testingId>[^\\/]+)/testing-endpoint$"
					],
					"strip_path": false,
					"plugins": [
						{
							"name": "request-validator",
							"config": {
								"version": "draft4",
								"parameter_schema": [
									{
										"in": "path",
										"explode": false,
										"required": true,
										"name": "testingId",
										"schema": "{\"type\":\"string\",\"maxLength\":50}",
										"style": "simple"
									}
								],
								"body_schema": "{\"description\":\"desc\",\"allOf\":[{\"$ref\":\"#/components/schemas/Entity1\"}],\"components\":{\"schemas\":{\"Entity1\":{\"description\":\"desc\",\"type\":\"object\",\"properties\":{\"entity1Prop1\":{\"$ref\":\"#/components/schemas/Entity2\"}}},\"Entity2\":{\"description\":\"desc\",\"oneOf\":[{\"$ref\":\"#/components/schemas/EntityCircularStart\"}],\"discriminator\":{\"propertyName\":\"entity5Type\",\"mapping\":{\"ENTITY_CIRCULAR_START\":\"#/components/schemas/EntityCircularStart\"}}},\"EntityCircularStart\":{\"description\":\"desc\",\"allOf\":[{\"$ref\":\"#/components/schemas/Entity5Common\"},{\"$ref\":\"#/components/schemas/Entity4Common\"}]},\"Entity5Common\":{\"description\":\"desc\",\"type\":\"string\"},\"Entity4Common\":{\"description\":\"desc\",\"type\":\"string\",\"properties\":{\"owners\":{\"$ref\":\"#/components/schemas/Entity4Owners\"}}},\"Entity4Owners\":{\"type\":\"array\",\"description\":\"desc\",\"items\":{\"$ref\":\"#/components/schemas/Entity6\"}},\"Entity6\":{\"description\":\"desc\",\"type\":\"object\",\"required\":[\"entity5\"],\"properties\":{\"entity5\":{\"$ref\":\"#/components/schemas/Entity5AsOwner\"}}},\"Entity5AsOwner\":{\"description\":\"desc\",\"oneOf\":[{\"$ref\":\"#/components/schemas/Entity3AsOwner\"}],\"discriminator\":{\"propertyName\":\"entity5Type\",\"mapping\":{\"ENTITY_3\":\"#/components/schemas/Entity3AsOwner\"}}},\"Entity3AsOwner\":{\"description\":\"desc\",\"allOf\":[{\"$ref\":\"#/components/schemas/Entity3Common\"}]},\"Entity3Common\":{\"description\":\"desc\",\"allOf\":[{\"$ref\":\"#/components/schemas/Entity5Common\"},{\"$ref\":\"#/components/schemas/Entity4Common\"}]}}},\"$schema\":\"https://json-schema.org/schema#\"}",
								"allowed_content_types": [
									"application/json"
								],
								"verbose_response": true
							},
							"tags": [
								"OAS3_import"
							],
							"enabled": true
						}
					]
				}
			],
			"tags": [
				"OAS3_import"
			]
		}
	]
}

[dimitropoulos]

@marckong and I just tested this in every way we know how, including with the user-provided original (multi-thousand line) YAML document that began this investigation.

We definitely identified a lot of areas for improvement, but at this juncture, it looks like it does what it needs to do and this is a good place to "draw the line in the sand" considering we have test coverage for this now and all the manual testing.