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

[Security Solution] Support bundling ESS and Serverless OAS separately #184348

Merged

Conversation

maximpn
Copy link
Contributor

@maximpn maximpn commented May 28, 2024

Resolves: https://github.com/elastic/security-team/issues/9516

Summary

As a part of Serverless API reference documentation effort we need to have an ability to produce independent Serverless and ESS OpenAPI specification (OAS) bundles. This PR addresses this issue by adding a new custom property x-labels (applicable to OAS operation objects) representing an array of strings and bundling configuration option to exclude anything marked with specific labels.

How does it work?

Added functionality allows to mark OAS operation object (objects defined under an API endpoint path as a HTTP method like get, post and etc) with arbitrary labels by using x-labels custom property like in an example below

paths:
  /api/some_path:
    get:
      x-labels:
        - label1
        - label2

This labelling DOESN'T change produced bundle by itself. It's required to use bundler's includeLabels option to include API endpoint operation object(s). includeLabels accepts a list of labels. An operation object is included when it has a label matching labels passed to includeLabels. In mathematical terms operation object's labels set intersects with includeLabels.

How to use it for producing separate Serverless and ESS bundles?

  • Mark OAS operation objects (HTTP methods like get or post) with x-labels custom property.

An example below has all operation objects under /api/some_path path labeled with ess label as well as operation objects under /api/another_path path. On top of that GET /api/another_path has serverless label as well.

...
paths:
  /api/some_path:
    get:
      x-labels: [ess]
      ...
    post:
      x-labels: [ess]
      ...
  /api/another_path:
    get:
      x-labels: [ess, serverless]
      ...
    post:
      x-labels: [ess]
      ...
...
  • Configure bundler with bundling options to include specific labels. options.includeLabels is responsible for including document nodes labeled with specific labels. You need two bundler invocations with different options.includeLabels values like below
bundle({ // (1)
  ...
  options: {
    includeLabels: ['serverless'],
  },
});

bundle({ // (2)
  ...
  options: {
    includeLabels: ['ess'],
  },
});

It will produce two following bundles

(1) for Serverless

...
paths:
  /api/another_path:
    get:
      ...
...

and (2) for ESS

...
paths:
  /api/some_path:
    get:
      ...
    post:
      ...
  /api/another_path:
    get:
      ...
    post:
      ...
...

You may notice (2) has everything included since each operation object is labeled with ess label.

@maximpn maximpn added docs Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team Project:Serverless Work as part of the Serverless project for its initial release v8.15.0 labels May 28, 2024
@maximpn maximpn self-assigned this May 28, 2024
@maximpn maximpn force-pushed the support-bundling-ess-and-serverless-separately branch from c4602b8 to 712732f Compare May 28, 2024 12:17
@maximpn maximpn requested a review from xcrzx May 28, 2024 13:04
@maximpn maximpn marked this pull request as ready for review May 28, 2024 19:11
@maximpn maximpn requested a review from a team as a code owner May 28, 2024 19:11
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management)

@maximpn maximpn added the release_note:skip Skip the PR/issue when compiling release notes label May 29, 2024
Copy link
Contributor

@xcrzx xcrzx left a comment

Choose a reason for hiding this comment

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

As discussed during tech time, let's try to implement the includeLabels filter for the generator and make this setting required.

@maximpn maximpn marked this pull request as draft May 31, 2024 07:35
@maximpn
Copy link
Contributor Author

maximpn commented Jun 3, 2024

Discussed complexity of includeLabels implementation with @xcrzx and we decided to support x-labels at Operation object level only. Arbitrary node/level for x-lables will bring extra complexity since it requires document tree preprocessing step. On top of that there is a chance we won't need support for labelling arbitrary node anytime soon.

Details

For example there is a following document tree (represented as a binary tree for simplicity) where one of the second level node (root level is one, the next below it is two) is labeled by ESS. We need to have nodes outlined by a green line and exclude nodes outlined by a red line. While processing documents shouldRemove receives nodes in in-order traversal order and it's impossible to decided whether a node should be removed without preprocessing the document tree and "lifting up" labels.

Screenshot 2024-06-03 at 09 34 19

Having labels at the concrete level (Operation object) helps to significantly simplify the implementation.

@maximpn maximpn force-pushed the support-bundling-ess-and-serverless-separately branch 2 times, most recently from bdc76ef to 426976f Compare June 3, 2024 09:28
@maximpn maximpn marked this pull request as ready for review June 3, 2024 09:40
@maximpn maximpn requested a review from a team as a code owner June 3, 2024 09:40
@maximpn
Copy link
Contributor Author

maximpn commented Jun 3, 2024

Discussion on whether options.includeLabels should be optional or not leaned towards having it optional for now for increased flexibility. We plan to reuse the bundler for preparing OAS for code generation. But it requires a different configuration than bundling for documentation purposes.

@maximpn maximpn requested a review from xcrzx June 3, 2024 09:50
@maximpn maximpn force-pushed the support-bundling-ess-and-serverless-separately branch from 426976f to c3b58ce Compare June 3, 2024 11:19
@maximpn maximpn requested review from banderror and removed request for xcrzx June 3, 2024 11:19
@banderror banderror force-pushed the support-bundling-ess-and-serverless-separately branch from c3b58ce to 324f2e4 Compare June 4, 2024 17:18
@banderror
Copy link
Contributor

Discussion on whether options.includeLabels should be optional or not leaned towards having it optional for now for increased flexibility. We plan to reuse the bundler for preparing OAS for code generation. But it requires a different configuration than bundling for documentation purposes.

@maximpn What will be the default behavior if options.includeLabels is not passed as an argument? Could you add a note about this to the PR description?

Copy link
Contributor

@banderror banderror left a comment

Choose a reason for hiding this comment

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

Thank you @maximpn, I left a few suggestions and concerns. I think the implementation has a minor bug, and there's a few things to improve. Otherwise, overall it looks really good!

@maximpn maximpn requested a review from banderror June 6, 2024 14:23
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

✅ unchanged

History

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

cc @maximpn

Copy link
Contributor

@banderror banderror left a comment

Choose a reason for hiding this comment

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

@maximpn LGTM, thank you for the fixes ✅

@maximpn maximpn merged commit 2755b89 into elastic:main Jun 7, 2024
36 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jun 7, 2024
@maximpn maximpn deleted the support-bundling-ess-and-serverless-separately branch June 7, 2024 13:14
banderror added a commit that referenced this pull request Jun 11, 2024
**Epic:** elastic/security-team#9693
(internal)
**Partially addresses:**
elastic/entity-analytics#60 (internal)

## Summary

This PR fixes a few issues with the existing OpenAPI schemas for the
Entity Analytics API based on findings described in
elastic/entity-analytics#60 and the recent
developments of the [OpenAPI specs
bundler](elastic/security-team#9401):

- [x] Thanks to #184348, now we
can mark endpoints as being available in ESS, Serverless, or both
offerings by marking them with `x-labels`.
- [x] We can also explicitly mark internal endpoints with `x-internal:
true`. While this is not required when the endpoint's path starts with
`/internal`, I think it's still good to mark them explicitly, according
to `access` modifier in the route registration. This fixed an issue
where a couple of endpoints, while being in fact internal, were included
in the OAS bundle because their paths do not start with `/internal`:
`/api/risk_scores/calculation`, `/api/risk_scores/calculation/entity`,
`/engine/settings`, `/engine/status`.
- [x] Specified correct paths for `/internal/risk_score/engine/settings`
and `/internal/risk_score/engine/status` endpoints.

## How to test

Run the bundler:

```sh
cd x-pack/plugins/security_solution
yarn openapi:bundle
```

Check that the OpenAPI bundles located under the
`x-pack/plugins/security_solution/target/openapi` folder don't contain
any Entity Analytics endpoints or components.
szwarckonrad added a commit that referenced this pull request Jul 5, 2024
…API (#187261)

This PR adds OpenAPI schemas for Defend Workflows API endpoints that
previously didn't have them. Here are the changes made:
1. Added a schema for `/api/endpoint/isolate`, which is deprecated and
now redirects as a `308` to the new path
(`/api/endpoint/action/isolate`). It's tagged with `x-labels` as `ess`
only.
2. Added a schema for `/api/endpoint/unisolate`, which is deprecated and
now redirects as a `308` to the new path
(`/api/endpoint/action/unisolate`). It's tagged with `x-labels` as `ess`
only.
3. Added a schema for
`/api/endpoint/protection_updates_note/{package_policy_id}`.
4. Added `x-labels` field to all existing Defend Workflows API paths for
proper tagging.

For more information on `x-labels`, please refer to
#184348
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting docs Project:Serverless Work as part of the Serverless project for its initial release release_note:skip Skip the PR/issue when compiling release notes Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants