Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request validator plugin respects existing properties and generates required ones #3283

Merged
merged 8 commits into from Apr 20, 2021

Conversation

develohpanda
Copy link
Contributor

@develohpanda develohpanda commented Apr 14, 2021

The request-validator plugin has several options to be send under the config object. These parameters are:

  • parameter_schema
  • body_schema
  • allowed_content_types
  • verbose_response

Out of these four, the first three can be generated or be user-defined, and the fourth must be user defined.

In an OpenAPI spec, the user may specify an object with none or some of the config parameters. If a user defines a config parameter, then that config parameter will be used and not generated. If a parameter that can be generated is not defined, then O2K will attempt to generate.

Here is an example spec with the plugin defined on each operation. This is not an exhaustive example, but should server as a good base to begin testing from.

openapi: 3.0.2

info:
  title: Example
  version: 1.0.0

servers:
  - url: http://backend.com/path

paths:
  /params:
    get:
      x-kong-plugin-request-validator:
        enabled: true
        config: 
          verbose_response: true
      parameters:
        - in: path
          name: userId
          schema:
            type: integer
          required: true
  /body:
    post:
      x-kong-plugin-request-validator:
        enabled: true
        config: 
          verbose_response: true
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/jsonSchema'
          application/xml:
            schema:
              $ref: '#/components/schemas/xmlSchema'

components:
  schemas:
    jsonSchema:
      type: object
      properties:
        id:
          type: integer
        name:
          type: string
    xmlSchema:
      type: object
      properties:
        prop:
          type: integer

If O2K is unable to generate the body_schema (from the operation request JSON body) or parameter_schema (from the operation parameters), then a default "pass-all" config should be applied, where the body_schema is set to '{}'.

See the unit tests added, which checks of these rules. Also see FTI-2252. This doesn't address all of the requirements outlined in the FTI, there is a subsequent PR to support plugins (request validator or otherwise) at other levels of the spec (root, server, path, operation).

Fixes INS-457, closes #2867

@develohpanda develohpanda added the PA-openapi-2-kong Package: OpenAPI 2 Kong label Apr 14, 2021
@develohpanda develohpanda self-assigned this Apr 14, 2021
@@ -63,4 +63,244 @@ describe('plugins', () => {
});
});
});

describe('generateRequestValidatorPlugin()', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

@@ -42,6 +42,7 @@ export function generateService(
for (const routePath of Object.keys(api.paths)) {
const pathItem: OA3PathItem = api.paths[routePath];

// TODO: Add path plugins to route
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completed and comment removed in a subsequent PR

@develohpanda develohpanda marked this pull request as ready for review April 15, 2021 05:48
});
});

it('should throw error if no operation request body content defined', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure what the effect is here.

Consider 200 operations and a request-validator defined.

  • 100 POST ones expecting a body (and having it defined)
  • 100 GET ones not expecting a body (and hence not defined in the OAS)

Then this would throw errors on the 100 GETs, right? In that case a user cannot specify the root level plugin config, but needs to add 100 specific ones for the POST ones. That will become painful...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, should it silently continue if it can't find the appropriate properties? This is me writing a test for existing behaviour but can certainly change it if it's wrong

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. I think that makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e41e047 (#3283). I also updated the behavior of parameters_schema generation to not throw errors.

@develohpanda develohpanda merged commit 874f6c9 into develop Apr 20, 2021
@develohpanda develohpanda deleted the feature/o2k-request-plugins branch April 20, 2021 01:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PA-openapi-2-kong Package: OpenAPI 2 Kong
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] openapi-2-kong ignoring verbose_response property of request validation plugin when generating config
3 participants