Skip to content

[HTTP/OAS] Include Security Solution domain OAS to production docs#194132

Merged
maximpn merged 5 commits intoelastic:mainfrom
maximpn:publish-security-solution-oas
Oct 4, 2024
Merged

[HTTP/OAS] Include Security Solution domain OAS to production docs#194132
maximpn merged 5 commits intoelastic:mainfrom
maximpn:publish-security-solution-oas

Conversation

@maximpn
Copy link
Copy Markdown
Contributor

@maximpn maximpn commented Sep 26, 2024

Epic: https://github.com/elastic/security-team/issues/9401 (internal)

Summary

This PR includes Security Solution OpenAPI domain bundles into the production OpenAPI Kibana bundle. The result Kibana bundler is expected to be published to Bump.sh manually by @lcawl.

@maximpn maximpn added release_note:skip Skip the PR/issue when compiling release notes v9.0.0 docs Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. backport:prev-minor Feature:OAS Work or issues related to Core-provided mechanisms for generating OAS v8.16.0 labels Sep 26, 2024
@maximpn maximpn requested a review from lcawl September 26, 2024 12:54
@maximpn maximpn self-assigned this Sep 26, 2024
@maximpn maximpn marked this pull request as ready for review September 26, 2024 12:54
@elasticmachine
Copy link
Copy Markdown
Contributor

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

@jmikell821
Copy link
Copy Markdown
Member

Hey @maximpn — I'd like for @natasha-moore-elastic to review these when she returns from PTO on October 7th before we merge (we also have a virtual offsite that week). Please add her as a reviewer on any subsequent Kibana PRs for Security. Thank you!

@banderror
Copy link
Copy Markdown
Contributor

banderror commented Sep 30, 2024

Hey @maximpn — I'd like for @natasha-moore-elastic to review these when she returns from PTO on October 7th before we merge (we also have a virtual offsite that week). Please add her as a reviewer on any subsequent Kibana PRs for Security. Thank you!

Hey @jmikell821, I wonder if you had any specific concerns or reasons for reviewing this PR from the Docs side. According to my understanding, this PR is purely technical and it doesn't introduce any changes to OpenAPI specs - it just integrates the specs we already had with the Kibana mechanism that will allow these specs to be published on bump.sh.

@natasha-moore-elastic told us that she had reviewed all the specs and gave us 🟢 light. Quote:

Hey all, I've completed the review of the Security API specs, updating operation summaries and adding missing operation descriptions. I haven't had a chance to update the tags yet (per the convo here) but I don't think that's a blocker.

Ideally, we'd like the docs to be published sooner, so that work doesn't consume @maximpn's resources who needs to focus on a high priority product epic.

@jmikell821
Copy link
Copy Markdown
Member

Hey @banderror — once @lcawl approves, I think this will be good to go, thanks for checking! cc: @maximpn

@lcawl
Copy link
Copy Markdown
Member

lcawl commented Oct 1, 2024

I ran the linting rules on the output (both before and after applying overlays) and there are 55 messages about operation-tag-defined "Operation tags must be defined in global tags" in the security APIs. From the ones I looked at, it's operations that have multiple tags assigned and Bump.sh only heeds the first tag anyway so as long as that's the case for all 55 here, you are okay. However, it might make it harder for you to tell in the future when a true problem occurs (e.g. first tag is not in the global list of tags and thus that endpoint isn't published). It might be safest to just add all those extra tags to the global list.

I did a quick test by adding the following missing tags via an overlay:

  - target: '$.tags'
    description: Add missing tags
    update:
      - name: Tags API
      - name: Alerts API
      - name: Alerts migration API
      - name: Rule preview API
      - name: Prebuilt Rules API
      - name: Bulk API
      - name: Privileges API
      - name: Alert index API
      - name: Rules API
      - name: Import/Export API
      - name: access:securitySolution
      - name: AnonymizationFields API
      - name: Chat Complete API
      - name: Conversation API
      - name: Conversations API
      - name: Prompts API

That makes the linting messages disappear but doesn't change the navigation in Bump.sh (since none of these are the first or only tag assigned to an operation).

So IMO you can move forward either with:

a) add an overlay for this type of tags (and then continue to update it as new situations like this arise), or
b) update the merge_*_oas.js scripts to add all tags to the generated output (and just continue ensuring that the ones you want to use are the first ones in each operation's tag list), or
c) ensure that each operation has only a single tag (and it's the one you want to use in the docs) in the security YAML files

If you decide to use (a) and want my help with the overlay, lmk!

@maximpn
Copy link
Copy Markdown
Contributor Author

maximpn commented Oct 2, 2024

@lcawl

Tags intentionally organised that way. OpenAPI bundler adds domain level tags by assigning a tag specified via configuration to each API domain's operation and adds the same tags to document root tags (for example Security Detections API tag is defined here). Besides that source OAS files have some tags to label sub-domains (like here).

We decided that OpenAPI Bundler won't remove any existing tags. Instead it adds the tag(s) from configuration to the top of the list so existing tags don't impact how API reference documentation is shown on Bump.sh. According to OpenAPI Specification tags don't have to be added to the root tags list

A list of tags used by the specification with additional metadata. The order of the tags can be used to reflect on their order by the parsing tools. Not all tags that are used by the Operation Object must be declared. The tags that are not declared MAY be organized randomly or based on the tools' logic. Each tag name in the list MUST be unique.


Does Bump.sh plan to support multiple tags per operation?

Could we use an overlay to implement option c)?

@lcawl
Copy link
Copy Markdown
Member

lcawl commented Oct 3, 2024

Does Bump.sh plan to support multiple tags per operation?

We've initiated discussions about that, in particular related to the use of tags in the Elastic Cloud APIs. However, we don't know yet when/if that will be added to the roadmap.

Could we use an overlay to implement option c)?

I added that via c3083805a8b21f86589c50fafaf0a7fe508d7045 and it seems to work. I also added some x-displayName overlays so these security tags are more consistent with the rest of the tags. We can remove those as needed when the Security writers continue their tag reviews.

Copy link
Copy Markdown
Member

@lcawl lcawl left a comment

Choose a reason for hiding this comment

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

If you're happy with the overlays I added, the output LGTM

@maximpn maximpn requested review from a team as code owners October 3, 2024 09:38
Copy link
Copy Markdown
Contributor

@tiansivive tiansivive left a comment

Choose a reason for hiding this comment

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

LGTM from Entity Analytics

@maximpn maximpn force-pushed the publish-security-solution-oas branch from 8c63866 to 0651d60 Compare October 3, 2024 10:41
@maximpn maximpn requested a review from a team as a code owner October 3, 2024 10:41
Copy link
Copy Markdown
Member

@spong spong left a comment

Choose a reason for hiding this comment

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

Security Assistant changes (addition of x-displayName) LGTM 👍

Thanks @maximpn!

@lcawl
Copy link
Copy Markdown
Member

lcawl commented Oct 3, 2024

... I simplified it via add x-displayName to Security Solution domain OAS bundles in 8c63866.

Great! That's better than maintaining overlays.

I came up to something like you added to remove all but first tags via overlay ... but I wasn't able to verify it locally. make api-docs-overlay run from oas_docs folder just hands for me. How do you run it? From what folder?

Hm, I had hoped my use of npx in the makefile meant folks wouldn't have to install the command lines, but it sounds like until this is integrated into buildkite, that's not the case. You might have to install Bump.sh (for overlaying and previewing output): https://docs.bump.sh/help/continuous-integration/cli/#global-installation, Spectral (for linting the documents): https://docs.stoplight.io/docs/spectral/b8391e051b7d8-installation and Redocly (for bundling the examples into the OpenAPI files): https://redocly.com/docs/cli/installation

By the way, when I run the make-overlay-docs, it's objecting to this info that now appears in the serverless bundle:

      servers: !<tag:yaml.org,2002:js/undefined> ''
      security: !<tag:yaml.org,2002:js/undefined> ''

That wasn't there when I tested yesterday so I assume it must be related to the latest updates?

@maximpn maximpn force-pushed the publish-security-solution-oas branch from 3efb80a to 8e034b3 Compare October 3, 2024 14:47
@maximpn
Copy link
Copy Markdown
Contributor Author

maximpn commented Oct 3, 2024

@lcawl

Hm, I had hoped my use of npx in the makefile meant folks wouldn't have to install the command lines, but it sounds like until this is integrated into buildkite, that's not the case. You might have to install Bump.sh (for overlaying and previewing output): https://docs.bump.sh/help/continuous-integration/cli/#global-installation, Spectral (for linting the documents): https://docs.stoplight.io/docs/spectral/b8391e051b7d8-installation and Redocly (for bundling the examples into the OpenAPI files): https://redocly.com/docs/cli/installation

Thanks. Everything but Bump.sh installed globally so it was the issue. After I installed Bump.sh globally make api-docs-overlay started working as expected.


By the way, when I run the make-overlay-docs, it's objecting to this info that now appears in the serverless ... That wasn't there when I tested yesterday so I assume it must be related to the latest updates?

I tried to find servers: !<tag:yaml.org,2002:js/undefined> '' or security: !<tag:yaml.org,2002:js/undefined> '' in Kibana repo but wasn't able to. Did you re-bundle some OAS locally?

Quite recently js-yaml was updated to v4. In case you didn't update Kibana deps you may have described issues. Could you try first running yarn kbn bootstrap and re-check make-overlay-docs doesn't complain on servers: !<tag:yaml.org,2002:js/undefined> '' or security: !<tag:yaml.org,2002:js/undefined> ''?

@lcawl
Copy link
Copy Markdown
Member

lcawl commented Oct 3, 2024

Could you try first running yarn kbn bootstrap and re-check make-overlay-docs doesn't complain on servers: !<tag:yaml.org,2002:js/undefined> '' or security: !<tag:yaml.org,2002:js/undefined> ''?

Good call, that fixed the issue. LGTM!

@kibana-ci
Copy link
Copy Markdown

💛 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
Copy Markdown
Contributor

@janmonschke janmonschke left a comment

Choose a reason for hiding this comment

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

THI changes lgtm 👍

@maximpn maximpn removed the request for review from dhurley14 October 4, 2024 09:09
x-displayName: Security exceptions
- description: Lists API allows you to manage lists of keywords, IPs or IP ranges items.
name: Security Lists API
x-displayName: Security lists
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@approksiu @nastasha-solomon do we refer to them as Security value lists or just Security lists?

Copy link
Copy Markdown
Member

@nastasha-solomon nastasha-solomon Oct 4, 2024

Choose a reason for hiding this comment

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

In the UI docs, we use value lists: https://www.elastic.co/guide/en/security/current/value-lists-exceptions.html

In the API docs, we use lists: https://www.elastic.co/guide/en/security/current/lists-index-api-overview.html

Also, I'm not sure whether you need to specify that these lists are for Security only. The distinction might be helpful if you can add exceptions to Kibana and Observability rules, and those exceptions can use lists. Otherwise, value lists or lists should suffice.

Copy link
Copy Markdown
Contributor

@yctercero yctercero left a comment

Choose a reason for hiding this comment

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

DE looks good to me - thanks!

I don't want to block, but did just leave one question about aligning our docs with whether we use value lists or lists. Could be followed up on.

@maximpn maximpn merged commit 102297c into elastic:main Oct 4, 2024
@maximpn maximpn deleted the publish-security-solution-oas branch October 4, 2024 19:34
@kibanamachine
Copy link
Copy Markdown
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11185902657

@kibanamachine
Copy link
Copy Markdown
Contributor

💔 All backports failed

Status Branch Result
8.x Backport failed because of merge conflicts

You might need to backport the following PRs to 8.x:
- Set spaces and roles CRUD APIs to public (#193534)

Manual backport

To create the backport manually run:

node scripts/backport --pr 194132

Questions ?

Please refer to the Backport tool documentation

@maximpn
Copy link
Copy Markdown
Contributor Author

maximpn commented Oct 7, 2024

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

maximpn added a commit to maximpn/kibana that referenced this pull request Oct 7, 2024
…lastic#194132)

*Epic:** elastic/security-team#9401 (internal)

## Summary

This PR includes Security Solution OpenAPI domain bundles into the production OpenAPI Kibana bundle. The result Kibana bundler is expected to be published to Bump.sh manually by @lcawl.

(cherry picked from commit 102297c)

# Conflicts:
#	oas_docs/output/kibana.serverless.staging.yaml
#	oas_docs/output/kibana.serverless.yaml
#	oas_docs/output/kibana.staging.yaml
#	oas_docs/output/kibana.yaml
#	x-pack/plugins/security_solution/docs/openapi/ess/security_solution_entity_analytics_api_1.bundled.schema.yaml
#	x-pack/plugins/security_solution/docs/openapi/serverless/security_solution_entity_analytics_api_1.bundled.schema.yaml
tiansivive pushed a commit to tiansivive/kibana that referenced this pull request Oct 7, 2024
…lastic#194132)

*Epic:** elastic/security-team#9401 (internal)

## Summary

This PR includes Security Solution OpenAPI domain bundles into the production OpenAPI Kibana bundle. The result Kibana bundler is expected to be published to Bump.sh manually by @lcawl.
maximpn added a commit that referenced this pull request Oct 7, 2024
…ocs (#194132) (#195221)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[HTTP/OAS] Include Security Solution domain OAS to production docs
(#194132)](#194132)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Maxim
Palenov","email":"maxim.palenov@elastic.co"},"sourceCommit":{"committedDate":"2024-10-04T19:34:25Z","message":"[HTTP/OAS]
Include Security Solution domain OAS to production docs
(#194132)\n\n*Epic:**
elastic/security-team#9401
(internal)\r\n\r\n## Summary\r\n\r\nThis PR includes Security Solution
OpenAPI domain bundles into the production OpenAPI Kibana bundle. The
result Kibana bundler is expected to be published to Bump.sh manually by
@lcawl.","sha":"102297ca151d56c8a7da36c14c72386b4cd225ca","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","docs","Team:
SecuritySolution","backport:prev-minor","Feature:OAS","v8.16.0"],"number":194132,"url":"https://github.com/elastic/kibana/pull/194132","mergeCommit":{"message":"[HTTP/OAS]
Include Security Solution domain OAS to production docs
(#194132)\n\n*Epic:**
elastic/security-team#9401
(internal)\r\n\r\n## Summary\r\n\r\nThis PR includes Security Solution
OpenAPI domain bundles into the production OpenAPI Kibana bundle. The
result Kibana bundler is expected to be published to Bump.sh manually by
@lcawl.","sha":"102297ca151d56c8a7da36c14c72386b4cd225ca"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/194132","number":194132,"mergeCommit":{"message":"[HTTP/OAS]
Include Security Solution domain OAS to production docs
(#194132)\n\n*Epic:**
elastic/security-team#9401
(internal)\r\n\r\n## Summary\r\n\r\nThis PR includes Security Solution
OpenAPI domain bundles into the production OpenAPI Kibana bundle. The
result Kibana bundler is expected to be published to Bump.sh manually by
@lcawl.","sha":"102297ca151d56c8a7da36c14c72386b4cd225ca"}},{"branch":"8.x","label":"v8.16.0","labelRegex":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Feature:OAS Work or issues related to Core-provided mechanisms for generating OAS release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.16.0 v9.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.