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

[OAS PoC]OAS endpoint and lazily pass schema #164710

Conversation

jloleysens
Copy link
Contributor

@jloleysens jloleysens commented Aug 24, 2023

Summary

Diverges a little bit from the original PoC but largely remains the same. The idea is to support lazily providing HTTP route schemas and also provide the ability to serve up OAS from an HTTP endpoint.

  • expose the generate OAS function so that it can be used from an endpoint created in core's HTTP server
  • create the ability to pass schema lazily via versioned router interface so that it can be constructed after setup/start
  • use the new lazy schema option in alerting, see x-pack/plugins/alerting/server/routes/update_rule.ts
  • also removes the CLI node script/generate_oas in favor of the new endpoint for now

Much of this can also be done for existing, non-versioned and versioned non-zod API routes if we add support for Joi.

TODO

  • Needs to be tested end-to-end, currently the lazily loaded schemas will not be used during actual route validation

How to test

  1. Start ES & Kibana
  2. Issue a request like: curl -uelastic:changeme http://localhost:5601/<base-path-thingy>/api/oas (optionally format with jq for legibility)
  3. Should see a fairly incomplete OAS doc bc zod is not being used in any endpoints, but you should see one of the lazily added schemas:
    "/api/alerting/rule/{id}": {
      "put": {
        "requestBody": {
          "content": {
            "application/json; Elastic-Api-Version=2023-10-31": {
              "schema": {
                "type": "object",
                "properties": {
                  "data": {
                    "type": "string",
                    "description": "I arrived lazily"
                  }
                },
                "required": [
                  "data"
                ],
                "additionalProperties": false
              }
            }
          }
        },
        "responses": {},
        "parameters": [
          {
            "name": "id",
            "in": "path",
            "required": true,
            "schema": {
              "type": "string"
            }
          }
        ],
        "operationId": "/api/alerting/rule/{id}#0"
      }
    }

@jloleysens jloleysens changed the title [OAS] OAS endpoint and lazily pass schema [OAS PoC]OAS endpoint and lazily pass schema Aug 24, 2023
Comment on lines 133 to 137
validate: () => ({
request: {
body: z.object({ data: z.string().describe('this is a placeholder!') }),
params: z.object({ id: z.string() }),
},
Copy link
Contributor Author

@jloleysens jloleysens Aug 24, 2023

Choose a reason for hiding this comment

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

I am guessing we can lazily load the alert's params schemas here.

@@ -251,7 +251,7 @@ export interface AddVersionOpts<P, Q, B> {
* Validation for this version of a route
* @experimental
*/
validate: false | FullValidationConfig<P, Q, B>;
validate: false | FullValidationConfig<P, Q, B> | (() => FullValidationConfig<P, Q, B>); // Provide a way to lazily load validation schemas
Copy link
Contributor Author

@jloleysens jloleysens Aug 24, 2023

Choose a reason for hiding this comment

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

Very naive approach to implementation taken here, perhaps the lazily loaded schemas should be cached? But good enough for PoC

@jloleysens jloleysens marked this pull request as ready for review August 25, 2023 10:11
@jloleysens jloleysens requested review from a team as code owners August 25, 2023 10:11
@jloleysens jloleysens merged commit 2f0ea25 into elastic:versioned-router-and-oas Aug 25, 2023
@jloleysens jloleysens deleted the versioned-router-and-oas-route branch August 25, 2023 10:12
@kibana-ci
Copy link
Collaborator

kibana-ci commented Aug 25, 2023

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #59 / Actions and Triggers app Rule Details Edit rule button should open edit rule flyout
  • [job] [logs] FTR Configs #59 / Actions and Triggers app Rule Details Edit rule button should open edit rule flyout
  • [job] [logs] x-pack/test/alerting_api_integration/security_and_spaces/group2/config_non_dedicated_task_runner.ts / alerting api integration security and spaces enabled - Group 2 Alerts legacy alerts alerts superuser at space1 should schedule actions on legacy alerts
  • [job] [logs] x-pack/test/alerting_api_integration/security_and_spaces/group2/config.ts / alerting api integration security and spaces enabled - Group 2 Alerts legacy alerts alerts superuser at space1 should schedule actions on legacy alerts
  • [job] [logs] x-pack/test/alerting_api_integration/security_and_spaces/group2/config_non_dedicated_task_runner.ts / alerting api integration security and spaces enabled - Group 2 Alerts legacy alerts alerts superuser at space1 should schedule actions on legacy alerts
  • [job] [logs] x-pack/test/alerting_api_integration/security_and_spaces/group2/config.ts / alerting api integration security and spaces enabled - Group 2 Alerts legacy alerts alerts superuser at space1 should schedule actions on legacy alerts
  • [job] [logs] FTR Configs #15 / alerting api integration security and spaces enabled - Group 3 Alerts - Group 3 alerts user managed api key rule operations should successfully update rule with user managed API key
  • [job] [logs] FTR Configs #15 / alerting api integration security and spaces enabled - Group 3 Alerts - Group 3 alerts user managed api key rule operations should successfully update rule with user managed API key
  • [job] [logs] FTR Configs #31 / Alerting update should handle update alert request appropriately
  • [job] [logs] FTR Configs #31 / Alerting update should handle update alert request appropriately
  • [job] [logs] FTR Configs #27 / apis index_patterns index_patterns/_fields_for_wildcard route conflicts flags fields with mismatched types as conflicting
  • [job] [logs] FTR Configs #27 / apis index_patterns index_patterns/_fields_for_wildcard route conflicts flags fields with mismatched types as conflicting
  • [job] [logs] FTR Configs #13 / Discover alerting Search source Alert should display actual state after rule params update on clicking viewInApp link
  • [job] [logs] FTR Configs #13 / Discover alerting Search source Alert should display actual state after rule params update on clicking viewInApp link
  • [job] [logs] Serverless Observability Tests / serverless common API Alerting APIs Alerting rules should pass updated rule params to executor
  • [job] [logs] Serverless Search Tests / serverless common API Alerting APIs Alerting rules should pass updated rule params to executor
  • [job] [logs] Serverless Security Tests / serverless common API Alerting APIs Alerting rules should pass updated rule params to executor
  • [job] [logs] Jest Tests #2 / updateRuleRoute ensures the license allows updating rules
  • [job] [logs] Jest Tests #2 / updateRuleRoute ensures the license check prevents updating rules
  • [job] [logs] Jest Tests #2 / updateRuleRoute ensures the rule type gets validated for the license
  • [job] [logs] Jest Tests #2 / updateRuleRoute updates a rule with proper parameters

Metrics [docs]

‼️ ERROR: no builds found for mergeBase sha [e582640]

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants